| Message ID | 20180828003844.4682-2-anusha.srivatsa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] firmware/dmc/icl: load v1.07 on icelake. | expand |
On Mon, Aug 27, 2018 at 05:38:44PM -0700, Anusha Srivatsa wrote: > Add Support to load DMC on Icelake. > > While at it, also add support to load the firmware > during system resume. > > v2: load firmware during system resume.(Imre) > > v3: enable has_csr for icelake.(Jyoti) > > v4: Only load the firmware in this patch > > Cc: Jyoti Yadav <jyoti.r.yadav@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> Reviewed-by: Imre Deak <imre.deak@intel.com> Is it ok to push this already now that the ICL 1.07 firmware is in [1] or do we have to wait until it propagates to [2]? [1] https://cgit.freedesktop.org/drm/drm-firmware/ [2] https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git > --- > drivers/gpu/drm/i915/intel_csr.c | 7 +++++++ > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++ > 2 files changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > index 1ec4f09c61f6..6d9d47322405 100644 > --- a/drivers/gpu/drm/i915/intel_csr.c > +++ b/drivers/gpu/drm/i915/intel_csr.c > @@ -34,6 +34,9 @@ > * low-power state and comes back to normal. > */ > > +#define I915_CSR_ICL "i915/icl_dmc_ver1_07.bin" > +#define ICL_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) > + > #define I915_CSR_GLK "i915/glk_dmc_ver1_04.bin" > MODULE_FIRMWARE(I915_CSR_GLK); > #define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 4) > @@ -301,6 +304,8 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, > if (csr->fw_path == i915_modparams.dmc_firmware_path) { > /* Bypass version check for firmware override. */ > required_version = csr->version; > + } else if (IS_ICELAKE(dev_priv)) { > + required_version = ICL_CSR_VERSION_REQUIRED; > } else if (IS_CANNONLAKE(dev_priv)) { > required_version = CNL_CSR_VERSION_REQUIRED; > } else if (IS_GEMINILAKE(dev_priv)) { > @@ -458,6 +463,8 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) > > if (i915_modparams.dmc_firmware_path) > csr->fw_path = i915_modparams.dmc_firmware_path; > + else if (IS_ICELAKE(dev_priv)) > + csr->fw_path = I915_CSR_ICL; > else if (IS_CANNONLAKE(dev_priv)) > csr->fw_path = I915_CSR_CNL; > else if (IS_GEMINILAKE(dev_priv)) > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 2852395125cd..bd7da068e813 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -3563,6 +3563,9 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv, > > /* 7. Setup MBUS. */ > icl_mbus_init(dev_priv); > + > + if (resume && dev_priv->csr.dmc_payload) > + intel_csr_load_program(dev_priv); > } > > static void icl_display_core_uninit(struct drm_i915_private *dev_priv) > -- > 2.17.1 >
On Mon, Sep 03, 2018 at 01:00:39PM +0300, Imre Deak wrote: > On Mon, Aug 27, 2018 at 05:38:44PM -0700, Anusha Srivatsa wrote: > > Add Support to load DMC on Icelake. > > > > While at it, also add support to load the firmware > > during system resume. > > > > v2: load firmware during system resume.(Imre) > > > > v3: enable has_csr for icelake.(Jyoti) > > > > v4: Only load the firmware in this patch > > > > Cc: Jyoti Yadav <jyoti.r.yadav@intel.com> > > Cc: Imre Deak <imre.deak@intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > > Reviewed-by: Imre Deak <imre.deak@intel.com> > > Is it ok to push this already now that the ICL 1.07 firmware is in [1] > or do we have to wait until it propagates to [2]? The main motivation behind having drm-firmware is the unpredictability of linux-firmware.git pull requests acceptance. It may take 1 day or it may take 2 months. So on drm-firmware we at least have it public in a way OSVs could easisly backport. Although hopefully by the end of 4.20 cycle I believe it will be there on linux-firmware.git already. So if fw is already on drm-firmware and passing all tests we should be able to push the patch to dinq. Thanks, Rodrigo. > > [1] https://cgit.freedesktop.org/drm/drm-firmware/ > [2] https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git > > > --- > > drivers/gpu/drm/i915/intel_csr.c | 7 +++++++ > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++ > > 2 files changed, 10 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > > index 1ec4f09c61f6..6d9d47322405 100644 > > --- a/drivers/gpu/drm/i915/intel_csr.c > > +++ b/drivers/gpu/drm/i915/intel_csr.c > > @@ -34,6 +34,9 @@ > > * low-power state and comes back to normal. > > */ > > > > +#define I915_CSR_ICL "i915/icl_dmc_ver1_07.bin" > > +#define ICL_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) > > + > > #define I915_CSR_GLK "i915/glk_dmc_ver1_04.bin" > > MODULE_FIRMWARE(I915_CSR_GLK); > > #define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 4) > > @@ -301,6 +304,8 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, > > if (csr->fw_path == i915_modparams.dmc_firmware_path) { > > /* Bypass version check for firmware override. */ > > required_version = csr->version; > > + } else if (IS_ICELAKE(dev_priv)) { > > + required_version = ICL_CSR_VERSION_REQUIRED; > > } else if (IS_CANNONLAKE(dev_priv)) { > > required_version = CNL_CSR_VERSION_REQUIRED; > > } else if (IS_GEMINILAKE(dev_priv)) { > > @@ -458,6 +463,8 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) > > > > if (i915_modparams.dmc_firmware_path) > > csr->fw_path = i915_modparams.dmc_firmware_path; > > + else if (IS_ICELAKE(dev_priv)) > > + csr->fw_path = I915_CSR_ICL; > > else if (IS_CANNONLAKE(dev_priv)) > > csr->fw_path = I915_CSR_CNL; > > else if (IS_GEMINILAKE(dev_priv)) > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index 2852395125cd..bd7da068e813 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -3563,6 +3563,9 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv, > > > > /* 7. Setup MBUS. */ > > icl_mbus_init(dev_priv); > > + > > + if (resume && dev_priv->csr.dmc_payload) > > + intel_csr_load_program(dev_priv); > > } > > > > static void icl_display_core_uninit(struct drm_i915_private *dev_priv) > > -- > > 2.17.1 > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Rodrigo Vivi (2018-09-04 08:27:14) > On Mon, Sep 03, 2018 at 01:00:39PM +0300, Imre Deak wrote: > > On Mon, Aug 27, 2018 at 05:38:44PM -0700, Anusha Srivatsa wrote: > > > Add Support to load DMC on Icelake. > > > > > > While at it, also add support to load the firmware > > > during system resume. > > > > > > v2: load firmware during system resume.(Imre) > > > > > > v3: enable has_csr for icelake.(Jyoti) > > > > > > v4: Only load the firmware in this patch > > > > > > Cc: Jyoti Yadav <jyoti.r.yadav@intel.com> > > > Cc: Imre Deak <imre.deak@intel.com> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > > Reviewed-by: Imre Deak <imre.deak@intel.com> > > > > Is it ok to push this already now that the ICL 1.07 firmware is in [1] > > or do we have to wait until it propagates to [2]? > > The main motivation behind having drm-firmware is the unpredictability > of linux-firmware.git pull requests acceptance. It may take 1 day or it > may take 2 months. > > So on drm-firmware we at least have it public in a way OSVs > could easisly backport. Although hopefully by the end of 4.20 > cycle I believe it will be there on linux-firmware.git already. > > So if fw is already on drm-firmware and passing all tests > we should be able to push the patch to dinq. Was not the decision that we only gate the MODULE_FIRMWARE line until the firmware is in linux-firmware.git? So it should be no harm to support loading firmwares that are available from drm-firmware.git as long as we don't add the MODULE_FIRMWARE which would trigger false positives for distro packagers. Regards, Joonas > > Thanks, > Rodrigo. > > > > > [1] https://cgit.freedesktop.org/drm/drm-firmware/ > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git > > > > > --- > > > drivers/gpu/drm/i915/intel_csr.c | 7 +++++++ > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++ > > > 2 files changed, 10 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > > > index 1ec4f09c61f6..6d9d47322405 100644 > > > --- a/drivers/gpu/drm/i915/intel_csr.c > > > +++ b/drivers/gpu/drm/i915/intel_csr.c > > > @@ -34,6 +34,9 @@ > > > * low-power state and comes back to normal. > > > */ > > > > > > +#define I915_CSR_ICL "i915/icl_dmc_ver1_07.bin" > > > +#define ICL_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) > > > + > > > #define I915_CSR_GLK "i915/glk_dmc_ver1_04.bin" > > > MODULE_FIRMWARE(I915_CSR_GLK); > > > #define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 4) > > > @@ -301,6 +304,8 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, > > > if (csr->fw_path == i915_modparams.dmc_firmware_path) { > > > /* Bypass version check for firmware override. */ > > > required_version = csr->version; > > > + } else if (IS_ICELAKE(dev_priv)) { > > > + required_version = ICL_CSR_VERSION_REQUIRED; > > > } else if (IS_CANNONLAKE(dev_priv)) { > > > required_version = CNL_CSR_VERSION_REQUIRED; > > > } else if (IS_GEMINILAKE(dev_priv)) { > > > @@ -458,6 +463,8 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) > > > > > > if (i915_modparams.dmc_firmware_path) > > > csr->fw_path = i915_modparams.dmc_firmware_path; > > > + else if (IS_ICELAKE(dev_priv)) > > > + csr->fw_path = I915_CSR_ICL; > > > else if (IS_CANNONLAKE(dev_priv)) > > > csr->fw_path = I915_CSR_CNL; > > > else if (IS_GEMINILAKE(dev_priv)) > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > index 2852395125cd..bd7da068e813 100644 > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > @@ -3563,6 +3563,9 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv, > > > > > > /* 7. Setup MBUS. */ > > > icl_mbus_init(dev_priv); > > > + > > > + if (resume && dev_priv->csr.dmc_payload) > > > + intel_csr_load_program(dev_priv); > > > } > > > > > > static void icl_display_core_uninit(struct drm_i915_private *dev_priv) > > > -- > > > 2.17.1 > > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Sep 05, 2018 at 12:07:43PM +0300, Joonas Lahtinen wrote: > Quoting Rodrigo Vivi (2018-09-04 08:27:14) > > On Mon, Sep 03, 2018 at 01:00:39PM +0300, Imre Deak wrote: > > > On Mon, Aug 27, 2018 at 05:38:44PM -0700, Anusha Srivatsa wrote: > > > > Add Support to load DMC on Icelake. > > > > > > > > While at it, also add support to load the firmware > > > > during system resume. > > > > > > > > v2: load firmware during system resume.(Imre) > > > > > > > > v3: enable has_csr for icelake.(Jyoti) > > > > > > > > v4: Only load the firmware in this patch > > > > > > > > Cc: Jyoti Yadav <jyoti.r.yadav@intel.com> > > > > Cc: Imre Deak <imre.deak@intel.com> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > > > > Reviewed-by: Imre Deak <imre.deak@intel.com> > > > > > > Is it ok to push this already now that the ICL 1.07 firmware is in [1] > > > or do we have to wait until it propagates to [2]? > > > > The main motivation behind having drm-firmware is the unpredictability > > of linux-firmware.git pull requests acceptance. It may take 1 day or it > > may take 2 months. > > > > So on drm-firmware we at least have it public in a way OSVs > > could easisly backport. Although hopefully by the end of 4.20 > > cycle I believe it will be there on linux-firmware.git already. > > > > So if fw is already on drm-firmware and passing all tests > > we should be able to push the patch to dinq. > > Was not the decision that we only gate the MODULE_FIRMWARE line until > the firmware is in linux-firmware.git? > > So it should be no harm to support loading firmwares that are available > from drm-firmware.git as long as we don't add the MODULE_FIRMWARE which > would trigger false positives for distro packagers. Ok, makes sense. I missed the lack of MODULE_FIRMWARE line during my review:/ but that's ok then, we should add that later enabling DMC already now in CI. Thanks, Imre > > Regards, Joonas > > > > > Thanks, > > Rodrigo. > > > > > > > > [1] https://cgit.freedesktop.org/drm/drm-firmware/ > > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git > > > > > > > --- > > > > drivers/gpu/drm/i915/intel_csr.c | 7 +++++++ > > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++ > > > > 2 files changed, 10 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > > > > index 1ec4f09c61f6..6d9d47322405 100644 > > > > --- a/drivers/gpu/drm/i915/intel_csr.c > > > > +++ b/drivers/gpu/drm/i915/intel_csr.c > > > > @@ -34,6 +34,9 @@ > > > > * low-power state and comes back to normal. > > > > */ > > > > > > > > +#define I915_CSR_ICL "i915/icl_dmc_ver1_07.bin" > > > > +#define ICL_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) > > > > + > > > > #define I915_CSR_GLK "i915/glk_dmc_ver1_04.bin" > > > > MODULE_FIRMWARE(I915_CSR_GLK); > > > > #define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 4) > > > > @@ -301,6 +304,8 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, > > > > if (csr->fw_path == i915_modparams.dmc_firmware_path) { > > > > /* Bypass version check for firmware override. */ > > > > required_version = csr->version; > > > > + } else if (IS_ICELAKE(dev_priv)) { > > > > + required_version = ICL_CSR_VERSION_REQUIRED; > > > > } else if (IS_CANNONLAKE(dev_priv)) { > > > > required_version = CNL_CSR_VERSION_REQUIRED; > > > > } else if (IS_GEMINILAKE(dev_priv)) { > > > > @@ -458,6 +463,8 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) > > > > > > > > if (i915_modparams.dmc_firmware_path) > > > > csr->fw_path = i915_modparams.dmc_firmware_path; > > > > + else if (IS_ICELAKE(dev_priv)) > > > > + csr->fw_path = I915_CSR_ICL; > > > > else if (IS_CANNONLAKE(dev_priv)) > > > > csr->fw_path = I915_CSR_CNL; > > > > else if (IS_GEMINILAKE(dev_priv)) > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > index 2852395125cd..bd7da068e813 100644 > > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > @@ -3563,6 +3563,9 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv, > > > > > > > > /* 7. Setup MBUS. */ > > > > icl_mbus_init(dev_priv); > > > > + > > > > + if (resume && dev_priv->csr.dmc_payload) > > > > + intel_csr_load_program(dev_priv); > > > > } > > > > > > > > static void icl_display_core_uninit(struct drm_i915_private *dev_priv) > > > > -- > > > > 2.17.1 > > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Sep 05, 2018 at 12:07:43PM +0300, Joonas Lahtinen wrote: > Quoting Rodrigo Vivi (2018-09-04 08:27:14) > > On Mon, Sep 03, 2018 at 01:00:39PM +0300, Imre Deak wrote: > > > On Mon, Aug 27, 2018 at 05:38:44PM -0700, Anusha Srivatsa wrote: > > > > Add Support to load DMC on Icelake. > > > > > > > > While at it, also add support to load the firmware > > > > during system resume. > > > > > > > > v2: load firmware during system resume.(Imre) > > > > > > > > v3: enable has_csr for icelake.(Jyoti) > > > > > > > > v4: Only load the firmware in this patch > > > > > > > > Cc: Jyoti Yadav <jyoti.r.yadav@intel.com> > > > > Cc: Imre Deak <imre.deak@intel.com> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > > > > Reviewed-by: Imre Deak <imre.deak@intel.com> > > > > > > Is it ok to push this already now that the ICL 1.07 firmware is in [1] > > > or do we have to wait until it propagates to [2]? > > > > The main motivation behind having drm-firmware is the unpredictability > > of linux-firmware.git pull requests acceptance. It may take 1 day or it > > may take 2 months. > > > > So on drm-firmware we at least have it public in a way OSVs > > could easisly backport. Although hopefully by the end of 4.20 > > cycle I believe it will be there on linux-firmware.git already. > > > > So if fw is already on drm-firmware and passing all tests > > we should be able to push the patch to dinq. > > Was not the decision that we only gate the MODULE_FIRMWARE line until > the firmware is in linux-firmware.git? > > So it should be no harm to support loading firmwares that are available > from drm-firmware.git as long as we don't add the MODULE_FIRMWARE which > would trigger false positives for distro packagers. As far as I can remember we had agreed on changing this and adding MODULE_FIRMWARE from the beginning. 1. the process gets complex 2. we forgot many times to add it afterwards 3. module_firmware changes nothing... only the fact that initrd generation wont complain if firmware is not there yet. 4. The old issue was that patches were merged without the pull request being sent... We fixed that by only accepting patches after pull request is sent. 5. By the time that all lands to distros linux-firmware.git will have the firmware because of 4. 6. We have now drm-firmware to mirror what official linux-firmware.git will be in few weeks from now. So I don't see a reason anymore why to keep with complicated process with split MODULE_FIRMWARE. Thanks, Rodrigo. > > Regards, Joonas > > > > > Thanks, > > Rodrigo. > > > > > > > > [1] https://cgit.freedesktop.org/drm/drm-firmware/ > > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git > > > > > > > --- > > > > drivers/gpu/drm/i915/intel_csr.c | 7 +++++++ > > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++ > > > > 2 files changed, 10 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > > > > index 1ec4f09c61f6..6d9d47322405 100644 > > > > --- a/drivers/gpu/drm/i915/intel_csr.c > > > > +++ b/drivers/gpu/drm/i915/intel_csr.c > > > > @@ -34,6 +34,9 @@ > > > > * low-power state and comes back to normal. > > > > */ > > > > > > > > +#define I915_CSR_ICL "i915/icl_dmc_ver1_07.bin" > > > > +#define ICL_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) > > > > + > > > > #define I915_CSR_GLK "i915/glk_dmc_ver1_04.bin" > > > > MODULE_FIRMWARE(I915_CSR_GLK); > > > > #define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 4) > > > > @@ -301,6 +304,8 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, > > > > if (csr->fw_path == i915_modparams.dmc_firmware_path) { > > > > /* Bypass version check for firmware override. */ > > > > required_version = csr->version; > > > > + } else if (IS_ICELAKE(dev_priv)) { > > > > + required_version = ICL_CSR_VERSION_REQUIRED; > > > > } else if (IS_CANNONLAKE(dev_priv)) { > > > > required_version = CNL_CSR_VERSION_REQUIRED; > > > > } else if (IS_GEMINILAKE(dev_priv)) { > > > > @@ -458,6 +463,8 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) > > > > > > > > if (i915_modparams.dmc_firmware_path) > > > > csr->fw_path = i915_modparams.dmc_firmware_path; > > > > + else if (IS_ICELAKE(dev_priv)) > > > > + csr->fw_path = I915_CSR_ICL; > > > > else if (IS_CANNONLAKE(dev_priv)) > > > > csr->fw_path = I915_CSR_CNL; > > > > else if (IS_GEMINILAKE(dev_priv)) > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > index 2852395125cd..bd7da068e813 100644 > > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > @@ -3563,6 +3563,9 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv, > > > > > > > > /* 7. Setup MBUS. */ > > > > icl_mbus_init(dev_priv); > > > > + > > > > + if (resume && dev_priv->csr.dmc_payload) > > > > + intel_csr_load_program(dev_priv); > > > > } > > > > > > > > static void icl_display_core_uninit(struct drm_i915_private *dev_priv) > > > > -- > > > > 2.17.1 > > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>-----Original Message----- >From: Vivi, Rodrigo >Sent: Monday, September 3, 2018 10:27 PM >To: Deak, Imre <imre.deak@intel.com> >Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; Nikula, Jani ><jani.nikula@intel.com>; intel-gfx@lists.freedesktop.org; Zanoni, Paulo R ><paulo.r.zanoni@intel.com> >Subject: Re: [Intel-gfx] [PATCH 1/1] firmware/dmc/icl: load v1.07 on icelake. > >On Mon, Sep 03, 2018 at 01:00:39PM +0300, Imre Deak wrote: >> On Mon, Aug 27, 2018 at 05:38:44PM -0700, Anusha Srivatsa wrote: >> > Add Support to load DMC on Icelake. >> > >> > While at it, also add support to load the firmware during system >> > resume. >> > >> > v2: load firmware during system resume.(Imre) >> > >> > v3: enable has_csr for icelake.(Jyoti) >> > >> > v4: Only load the firmware in this patch >> > >> > Cc: Jyoti Yadav <jyoti.r.yadav@intel.com> >> > Cc: Imre Deak <imre.deak@intel.com> >> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> >> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> >> >> Reviewed-by: Imre Deak <imre.deak@intel.com> >> >> Is it ok to push this already now that the ICL 1.07 firmware is in [1] >> or do we have to wait until it propagates to [2]? > >The main motivation behind having drm-firmware is the unpredictability of linux- >firmware.git pull requests acceptance. It may take 1 day or it may take 2 months. > >So on drm-firmware we at least have it public in a way OSVs could easisly >backport. Although hopefully by the end of 4.20 cycle I believe it will be there on >linux-firmware.git already. > >So if fw is already on drm-firmware and passing all tests we should be able to >push the patch to dinq. I will be sending the PR to linux-firmware.git. I think it is safe to push this patch. Regarding MODULE_FIRMWARE, Rodrigo, do you suggest I send that in a separate patch? In that case we can merge this patch as it is and have that as a separate one. In future, maybe better to add MODULE_FIRMWARE in the original patch? Anusha >Thanks, >Rodrigo. > >> >> [1] https://cgit.freedesktop.org/drm/drm-firmware/ >> [2] >> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmwar >> e.git >> >> > --- >> > drivers/gpu/drm/i915/intel_csr.c | 7 +++++++ >> > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++ >> > 2 files changed, 10 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_csr.c >> > b/drivers/gpu/drm/i915/intel_csr.c >> > index 1ec4f09c61f6..6d9d47322405 100644 >> > --- a/drivers/gpu/drm/i915/intel_csr.c >> > +++ b/drivers/gpu/drm/i915/intel_csr.c >> > @@ -34,6 +34,9 @@ >> > * low-power state and comes back to normal. >> > */ >> > >> > +#define I915_CSR_ICL "i915/icl_dmc_ver1_07.bin" >> > +#define ICL_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) >> > + >> > #define I915_CSR_GLK "i915/glk_dmc_ver1_04.bin" >> > MODULE_FIRMWARE(I915_CSR_GLK); >> > #define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 4) >> > @@ -301,6 +304,8 @@ static uint32_t *parse_csr_fw(struct >drm_i915_private *dev_priv, >> > if (csr->fw_path == i915_modparams.dmc_firmware_path) { >> > /* Bypass version check for firmware override. */ >> > required_version = csr->version; >> > + } else if (IS_ICELAKE(dev_priv)) { >> > + required_version = ICL_CSR_VERSION_REQUIRED; >> > } else if (IS_CANNONLAKE(dev_priv)) { >> > required_version = CNL_CSR_VERSION_REQUIRED; >> > } else if (IS_GEMINILAKE(dev_priv)) { @@ -458,6 +463,8 @@ void >> > intel_csr_ucode_init(struct drm_i915_private *dev_priv) >> > >> > if (i915_modparams.dmc_firmware_path) >> > csr->fw_path = i915_modparams.dmc_firmware_path; >> > + else if (IS_ICELAKE(dev_priv)) >> > + csr->fw_path = I915_CSR_ICL; >> > else if (IS_CANNONLAKE(dev_priv)) >> > csr->fw_path = I915_CSR_CNL; >> > else if (IS_GEMINILAKE(dev_priv)) >> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c >> > b/drivers/gpu/drm/i915/intel_runtime_pm.c >> > index 2852395125cd..bd7da068e813 100644 >> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c >> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c >> > @@ -3563,6 +3563,9 @@ static void icl_display_core_init(struct >> > drm_i915_private *dev_priv, >> > >> > /* 7. Setup MBUS. */ >> > icl_mbus_init(dev_priv); >> > + >> > + if (resume && dev_priv->csr.dmc_payload) >> > + intel_csr_load_program(dev_priv); >> > } >> > >> > static void icl_display_core_uninit(struct drm_i915_private >> > *dev_priv) >> > -- >> > 2.17.1 >> > >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Sep 05, 2018 at 11:55:32AM -0700, Srivatsa, Anusha wrote: > > > >-----Original Message----- > >From: Vivi, Rodrigo > >Sent: Monday, September 3, 2018 10:27 PM > >To: Deak, Imre <imre.deak@intel.com> > >Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; Nikula, Jani > ><jani.nikula@intel.com>; intel-gfx@lists.freedesktop.org; Zanoni, Paulo R > ><paulo.r.zanoni@intel.com> > >Subject: Re: [Intel-gfx] [PATCH 1/1] firmware/dmc/icl: load v1.07 on icelake. > > > >On Mon, Sep 03, 2018 at 01:00:39PM +0300, Imre Deak wrote: > >> On Mon, Aug 27, 2018 at 05:38:44PM -0700, Anusha Srivatsa wrote: > >> > Add Support to load DMC on Icelake. > >> > > >> > While at it, also add support to load the firmware during system > >> > resume. > >> > > >> > v2: load firmware during system resume.(Imre) > >> > > >> > v3: enable has_csr for icelake.(Jyoti) > >> > > >> > v4: Only load the firmware in this patch > >> > > >> > Cc: Jyoti Yadav <jyoti.r.yadav@intel.com> > >> > Cc: Imre Deak <imre.deak@intel.com> > >> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > >> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > >> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > >> > >> Reviewed-by: Imre Deak <imre.deak@intel.com> > >> > >> Is it ok to push this already now that the ICL 1.07 firmware is in [1] > >> or do we have to wait until it propagates to [2]? > > > >The main motivation behind having drm-firmware is the unpredictability of linux- > >firmware.git pull requests acceptance. It may take 1 day or it may take 2 months. > > > >So on drm-firmware we at least have it public in a way OSVs could easisly > >backport. Although hopefully by the end of 4.20 cycle I believe it will be there on > >linux-firmware.git already. > > > >So if fw is already on drm-firmware and passing all tests we should be able to > >push the patch to dinq. > > I will be sending the PR to linux-firmware.git. I think it is safe to push this patch. > > Regarding MODULE_FIRMWARE, Rodrigo, do you suggest I send that in a separate patch? In that case we can merge this patch as it is and have that as a separate one. yeap, let's keep a separated patch for now since we are not sure there is an rough consensus on getting it in one patch. > > In future, maybe better to add MODULE_FIRMWARE in the original patch? That is my idea. Besides those points that I raised on the other branch of this thread I remember of more two points in favor of having in only one patch: In case this patch here lands on 4.20, but linux-firmware.git takes 4 weeks to pull the firmware the 4.20 will have the fw support, but only 4.21 will have it fully supported with initrd installation etc. So, if in few months from now OSVs decide to free their distro on 4.20 they will have to remember to backport this patch or force the firmware to their initrd. But for sure by the time that OSVs get it the image itself will be already on linux-firmware.git And in unlikely case that it took months and months and firmware is not there yet causing the bad messages for OSVs than the issue is easily fixed on linux-firmware.git side. Or they pull from drm-firmware or they help us convincing linux-firmware.git to accept the pull. > > Anusha > >Thanks, > >Rodrigo. > > > >> > >> [1] https://cgit.freedesktop.org/drm/drm-firmware/ > >> [2] > >> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmwar > >> e.git > >> > >> > --- > >> > drivers/gpu/drm/i915/intel_csr.c | 7 +++++++ > >> > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++ > >> > 2 files changed, 10 insertions(+) > >> > > >> > diff --git a/drivers/gpu/drm/i915/intel_csr.c > >> > b/drivers/gpu/drm/i915/intel_csr.c > >> > index 1ec4f09c61f6..6d9d47322405 100644 > >> > --- a/drivers/gpu/drm/i915/intel_csr.c > >> > +++ b/drivers/gpu/drm/i915/intel_csr.c > >> > @@ -34,6 +34,9 @@ > >> > * low-power state and comes back to normal. > >> > */ > >> > > >> > +#define I915_CSR_ICL "i915/icl_dmc_ver1_07.bin" > >> > +#define ICL_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) > >> > + > >> > #define I915_CSR_GLK "i915/glk_dmc_ver1_04.bin" > >> > MODULE_FIRMWARE(I915_CSR_GLK); > >> > #define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 4) > >> > @@ -301,6 +304,8 @@ static uint32_t *parse_csr_fw(struct > >drm_i915_private *dev_priv, > >> > if (csr->fw_path == i915_modparams.dmc_firmware_path) { > >> > /* Bypass version check for firmware override. */ > >> > required_version = csr->version; > >> > + } else if (IS_ICELAKE(dev_priv)) { > >> > + required_version = ICL_CSR_VERSION_REQUIRED; > >> > } else if (IS_CANNONLAKE(dev_priv)) { > >> > required_version = CNL_CSR_VERSION_REQUIRED; > >> > } else if (IS_GEMINILAKE(dev_priv)) { @@ -458,6 +463,8 @@ void > >> > intel_csr_ucode_init(struct drm_i915_private *dev_priv) > >> > > >> > if (i915_modparams.dmc_firmware_path) > >> > csr->fw_path = i915_modparams.dmc_firmware_path; > >> > + else if (IS_ICELAKE(dev_priv)) > >> > + csr->fw_path = I915_CSR_ICL; > >> > else if (IS_CANNONLAKE(dev_priv)) > >> > csr->fw_path = I915_CSR_CNL; > >> > else if (IS_GEMINILAKE(dev_priv)) > >> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > >> > b/drivers/gpu/drm/i915/intel_runtime_pm.c > >> > index 2852395125cd..bd7da068e813 100644 > >> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > >> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > >> > @@ -3563,6 +3563,9 @@ static void icl_display_core_init(struct > >> > drm_i915_private *dev_priv, > >> > > >> > /* 7. Setup MBUS. */ > >> > icl_mbus_init(dev_priv); > >> > + > >> > + if (resume && dev_priv->csr.dmc_payload) > >> > + intel_csr_load_program(dev_priv); > >> > } > >> > > >> > static void icl_display_core_uninit(struct drm_i915_private > >> > *dev_priv) > >> > -- > >> > 2.17.1 > >> > > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>-----Original Message----- >From: Vivi, Rodrigo >Sent: Wednesday, September 5, 2018 12:31 PM >To: Srivatsa, Anusha <anusha.srivatsa@intel.com> >Cc: Deak, Imre <imre.deak@intel.com>; Nikula, Jani <jani.nikula@intel.com>; >intel-gfx@lists.freedesktop.org; Zanoni, Paulo R <paulo.r.zanoni@intel.com> >Subject: Re: [Intel-gfx] [PATCH 1/1] firmware/dmc/icl: load v1.07 on icelake. > >On Wed, Sep 05, 2018 at 11:55:32AM -0700, Srivatsa, Anusha wrote: >> >> >> >-----Original Message----- >> >From: Vivi, Rodrigo >> >Sent: Monday, September 3, 2018 10:27 PM >> >To: Deak, Imre <imre.deak@intel.com> >> >Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; Nikula, Jani >> ><jani.nikula@intel.com>; intel-gfx@lists.freedesktop.org; Zanoni, >> >Paulo R <paulo.r.zanoni@intel.com> >> >Subject: Re: [Intel-gfx] [PATCH 1/1] firmware/dmc/icl: load v1.07 on icelake. >> > >> >On Mon, Sep 03, 2018 at 01:00:39PM +0300, Imre Deak wrote: >> >> On Mon, Aug 27, 2018 at 05:38:44PM -0700, Anusha Srivatsa wrote: >> >> > Add Support to load DMC on Icelake. >> >> > >> >> > While at it, also add support to load the firmware during system >> >> > resume. >> >> > >> >> > v2: load firmware during system resume.(Imre) >> >> > >> >> > v3: enable has_csr for icelake.(Jyoti) >> >> > >> >> > v4: Only load the firmware in this patch >> >> > >> >> > Cc: Jyoti Yadav <jyoti.r.yadav@intel.com> >> >> > Cc: Imre Deak <imre.deak@intel.com> >> >> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >> >> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> >> >> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> >> >> >> >> Reviewed-by: Imre Deak <imre.deak@intel.com> >> >> >> >> Is it ok to push this already now that the ICL 1.07 firmware is in >> >> [1] or do we have to wait until it propagates to [2]? >> > >> >The main motivation behind having drm-firmware is the >> >unpredictability of linux- firmware.git pull requests acceptance. It may take 1 >day or it may take 2 months. >> > >> >So on drm-firmware we at least have it public in a way OSVs could >> >easisly backport. Although hopefully by the end of 4.20 cycle I >> >believe it will be there on linux-firmware.git already. >> > >> >So if fw is already on drm-firmware and passing all tests we should >> >be able to push the patch to dinq. >> >> I will be sending the PR to linux-firmware.git. I think it is safe to push this patch. >> >> Regarding MODULE_FIRMWARE, Rodrigo, do you suggest I send that in a >separate patch? In that case we can merge this patch as it is and have that as a >separate one. > >yeap, let's keep a separated patch for now since we are not sure there is an rough >consensus on getting it in one patch. Sure. >> >> In future, maybe better to add MODULE_FIRMWARE in the original patch? > >That is my idea. >Besides those points that I raised on the other branch of this thread I remember >of more two points in favor of having in only one patch: > >In case this patch here lands on 4.20, but linux-firmware.git takes >4 weeks to pull the firmware the 4.20 will have the fw support, but only 4.21 will >have it fully supported with initrd installation etc. > >So, if in few months from now OSVs decide to free their distro on 4.20 they will >have to remember to backport this patch or force the firmware to their initrd. But >for sure by the time that OSVs get it the image itself will be already on linux- >firmware.git > >And in unlikely case that it took months and months and firmware is not there yet >causing the bad messages for OSVs than the issue is easily fixed on linux- >firmware.git side. > >Or they pull from drm-firmware or they help us convincing linux-firmware.git to >accept the pull. That makes sense. Anusha >> >> Anusha >> >Thanks, >> >Rodrigo. >> > >> >> >> >> [1] https://cgit.freedesktop.org/drm/drm-firmware/ >> >> [2] >> >> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firm >> >> war >> >> e.git >> >> >> >> > --- >> >> > drivers/gpu/drm/i915/intel_csr.c | 7 +++++++ >> >> > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++ >> >> > 2 files changed, 10 insertions(+) >> >> > >> >> > diff --git a/drivers/gpu/drm/i915/intel_csr.c >> >> > b/drivers/gpu/drm/i915/intel_csr.c >> >> > index 1ec4f09c61f6..6d9d47322405 100644 >> >> > --- a/drivers/gpu/drm/i915/intel_csr.c >> >> > +++ b/drivers/gpu/drm/i915/intel_csr.c >> >> > @@ -34,6 +34,9 @@ >> >> > * low-power state and comes back to normal. >> >> > */ >> >> > >> >> > +#define I915_CSR_ICL "i915/icl_dmc_ver1_07.bin" >> >> > +#define ICL_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) >> >> > + >> >> > #define I915_CSR_GLK "i915/glk_dmc_ver1_04.bin" >> >> > MODULE_FIRMWARE(I915_CSR_GLK); >> >> > #define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 4) >> >> > @@ -301,6 +304,8 @@ static uint32_t *parse_csr_fw(struct >> >drm_i915_private *dev_priv, >> >> > if (csr->fw_path == i915_modparams.dmc_firmware_path) { >> >> > /* Bypass version check for firmware override. */ >> >> > required_version = csr->version; >> >> > + } else if (IS_ICELAKE(dev_priv)) { >> >> > + required_version = ICL_CSR_VERSION_REQUIRED; >> >> > } else if (IS_CANNONLAKE(dev_priv)) { >> >> > required_version = CNL_CSR_VERSION_REQUIRED; >> >> > } else if (IS_GEMINILAKE(dev_priv)) { @@ -458,6 +463,8 @@ void >> >> > intel_csr_ucode_init(struct drm_i915_private *dev_priv) >> >> > >> >> > if (i915_modparams.dmc_firmware_path) >> >> > csr->fw_path = i915_modparams.dmc_firmware_path; >> >> > + else if (IS_ICELAKE(dev_priv)) >> >> > + csr->fw_path = I915_CSR_ICL; >> >> > else if (IS_CANNONLAKE(dev_priv)) >> >> > csr->fw_path = I915_CSR_CNL; >> >> > else if (IS_GEMINILAKE(dev_priv)) diff --git >> >> > a/drivers/gpu/drm/i915/intel_runtime_pm.c >> >> > b/drivers/gpu/drm/i915/intel_runtime_pm.c >> >> > index 2852395125cd..bd7da068e813 100644 >> >> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c >> >> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c >> >> > @@ -3563,6 +3563,9 @@ static void icl_display_core_init(struct >> >> > drm_i915_private *dev_priv, >> >> > >> >> > /* 7. Setup MBUS. */ >> >> > icl_mbus_init(dev_priv); >> >> > + >> >> > + if (resume && dev_priv->csr.dmc_payload) >> >> > + intel_csr_load_program(dev_priv); >> >> > } >> >> > >> >> > static void icl_display_core_uninit(struct drm_i915_private >> >> > *dev_priv) >> >> > -- >> >> > 2.17.1 >> >> > >> >> _______________________________________________ >> >> Intel-gfx mailing list >> >> Intel-gfx@lists.freedesktop.org >> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, 05 Sep 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > On Wed, Sep 05, 2018 at 12:07:43PM +0300, Joonas Lahtinen wrote: >> Was not the decision that we only gate the MODULE_FIRMWARE line until >> the firmware is in linux-firmware.git? >> >> So it should be no harm to support loading firmwares that are available >> from drm-firmware.git as long as we don't add the MODULE_FIRMWARE which >> would trigger false positives for distro packagers. > > As far as I can remember we had agreed on changing this and adding > MODULE_FIRMWARE from the beginning. > > 1. the process gets complex > 2. we forgot many times to add it afterwards > 3. module_firmware changes nothing... only the fact that initrd generation > wont complain if firmware is not there yet. > 4. The old issue was that patches were merged without the pull request > being sent... We fixed that by only accepting patches after pull request > is sent. > 5. By the time that all lands to distros linux-firmware.git will have > the firmware because of 4. > 6. We have now drm-firmware to mirror what official linux-firmware.git will > be in few weeks from now. > > So I don't see a reason anymore why to keep with complicated process with > split MODULE_FIRMWARE. I've changed my mind, I agree with *not* splitting MODULE_FIRMWARE changes to another patch anymore. However, with that I think we need to redefine when we can add request_firmware and MODULE_FIRMWARE for a specific firmware blob. Here's my proposal. - Before merging a patch adding or changing a firmware blob in dinq, said blob must be publicly available at a known location, and no known blockers for linux-firmware.git merge may exist. - We take care to get the blob to linux-firmware.git before the changes hit Linus' -rc1 after the merge window. With our -rc5 feature deadline, this should give us at least 4-5 weeks to get the blob merged to linux-firmware.git. - It follows that linux-next and drm-tip may contain MODULE_FIRMWARE for blobs that don't exist in linux-firmware.git. BR, Jani.
Quoting Rodrigo Vivi (2018-09-05 19:42:28) > On Wed, Sep 05, 2018 at 12:07:43PM +0300, Joonas Lahtinen wrote: > > Quoting Rodrigo Vivi (2018-09-04 08:27:14) > > > On Mon, Sep 03, 2018 at 01:00:39PM +0300, Imre Deak wrote: > > > > On Mon, Aug 27, 2018 at 05:38:44PM -0700, Anusha Srivatsa wrote: > > > > > Add Support to load DMC on Icelake. > > > > > > > > > > While at it, also add support to load the firmware > > > > > during system resume. > > > > > > > > > > v2: load firmware during system resume.(Imre) > > > > > > > > > > v3: enable has_csr for icelake.(Jyoti) > > > > > > > > > > v4: Only load the firmware in this patch > > > > > > > > > > Cc: Jyoti Yadav <jyoti.r.yadav@intel.com> > > > > > Cc: Imre Deak <imre.deak@intel.com> > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > > > > > > Reviewed-by: Imre Deak <imre.deak@intel.com> > > > > > > > > Is it ok to push this already now that the ICL 1.07 firmware is in [1] > > > > or do we have to wait until it propagates to [2]? > > > > > > The main motivation behind having drm-firmware is the unpredictability > > > of linux-firmware.git pull requests acceptance. It may take 1 day or it > > > may take 2 months. > > > > > > So on drm-firmware we at least have it public in a way OSVs > > > could easisly backport. Although hopefully by the end of 4.20 > > > cycle I believe it will be there on linux-firmware.git already. > > > > > > So if fw is already on drm-firmware and passing all tests > > > we should be able to push the patch to dinq. > > > > Was not the decision that we only gate the MODULE_FIRMWARE line until > > the firmware is in linux-firmware.git? > > > > So it should be no harm to support loading firmwares that are available > > from drm-firmware.git as long as we don't add the MODULE_FIRMWARE which > > would trigger false positives for distro packagers. > > As far as I can remember we had agreed on changing this and adding > MODULE_FIRMWARE from the beginning. Is there an e-mail about it? I must have missed such discussion. > 1. the process gets complex > 2. we forgot many times to add it afterwards > 3. module_firmware changes nothing... only the fact that initrd generation > wont complain if firmware is not there yet. Yes, but isn't this the only thing we've got complaints of? At least I've not noticed distros complaining about not having the MODULE_FIRMWARE (which would omit the firmware from initrd). So I'd rather stick to the process previously decided on. Assuming pull request will be accepted in X days is always just an assumption. Regards, Joonas > 4. The old issue was that patches were merged without the pull request > being sent... We fixed that by only accepting patches after pull request > is sent. > 5. By the time that all lands to distros linux-firmware.git will have > the firmware because of 4. > 6. We have now drm-firmware to mirror what official linux-firmware.git will > be in few weeks from now. > > So I don't see a reason anymore why to keep with complicated process with > split MODULE_FIRMWARE. > > Thanks, > Rodrigo. > > > > > Regards, Joonas > > > > > > > > Thanks, > > > Rodrigo. > > > > > > > > > > > [1] https://cgit.freedesktop.org/drm/drm-firmware/ > > > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git > > > > > > > > > --- > > > > > drivers/gpu/drm/i915/intel_csr.c | 7 +++++++ > > > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++ > > > > > 2 files changed, 10 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > > > > > index 1ec4f09c61f6..6d9d47322405 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_csr.c > > > > > +++ b/drivers/gpu/drm/i915/intel_csr.c > > > > > @@ -34,6 +34,9 @@ > > > > > * low-power state and comes back to normal. > > > > > */ > > > > > > > > > > +#define I915_CSR_ICL "i915/icl_dmc_ver1_07.bin" > > > > > +#define ICL_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) > > > > > + > > > > > #define I915_CSR_GLK "i915/glk_dmc_ver1_04.bin" > > > > > MODULE_FIRMWARE(I915_CSR_GLK); > > > > > #define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 4) > > > > > @@ -301,6 +304,8 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, > > > > > if (csr->fw_path == i915_modparams.dmc_firmware_path) { > > > > > /* Bypass version check for firmware override. */ > > > > > required_version = csr->version; > > > > > + } else if (IS_ICELAKE(dev_priv)) { > > > > > + required_version = ICL_CSR_VERSION_REQUIRED; > > > > > } else if (IS_CANNONLAKE(dev_priv)) { > > > > > required_version = CNL_CSR_VERSION_REQUIRED; > > > > > } else if (IS_GEMINILAKE(dev_priv)) { > > > > > @@ -458,6 +463,8 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) > > > > > > > > > > if (i915_modparams.dmc_firmware_path) > > > > > csr->fw_path = i915_modparams.dmc_firmware_path; > > > > > + else if (IS_ICELAKE(dev_priv)) > > > > > + csr->fw_path = I915_CSR_ICL; > > > > > else if (IS_CANNONLAKE(dev_priv)) > > > > > csr->fw_path = I915_CSR_CNL; > > > > > else if (IS_GEMINILAKE(dev_priv)) > > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > > index 2852395125cd..bd7da068e813 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > > @@ -3563,6 +3563,9 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv, > > > > > > > > > > /* 7. Setup MBUS. */ > > > > > icl_mbus_init(dev_priv); > > > > > + > > > > > + if (resume && dev_priv->csr.dmc_payload) > > > > > + intel_csr_load_program(dev_priv); > > > > > } > > > > > > > > > > static void icl_display_core_uninit(struct drm_i915_private *dev_priv) > > > > > -- > > > > > 2.17.1 > > > > > > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Sep 06, 2018 at 09:46:13AM +0300, Jani Nikula wrote: > On Wed, 05 Sep 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > > On Wed, Sep 05, 2018 at 12:07:43PM +0300, Joonas Lahtinen wrote: > >> Was not the decision that we only gate the MODULE_FIRMWARE line until > >> the firmware is in linux-firmware.git? > >> > >> So it should be no harm to support loading firmwares that are available > >> from drm-firmware.git as long as we don't add the MODULE_FIRMWARE which > >> would trigger false positives for distro packagers. > > > > As far as I can remember we had agreed on changing this and adding > > MODULE_FIRMWARE from the beginning. > > > > 1. the process gets complex > > 2. we forgot many times to add it afterwards > > 3. module_firmware changes nothing... only the fact that initrd generation > > wont complain if firmware is not there yet. > > 4. The old issue was that patches were merged without the pull request > > being sent... We fixed that by only accepting patches after pull request > > is sent. > > 5. By the time that all lands to distros linux-firmware.git will have > > the firmware because of 4. > > 6. We have now drm-firmware to mirror what official linux-firmware.git will > > be in few weeks from now. > > > > So I don't see a reason anymore why to keep with complicated process with > > split MODULE_FIRMWARE. > > I've changed my mind, I agree with *not* splitting MODULE_FIRMWARE > changes to another patch anymore. > > However, with that I think we need to redefine when we can add > request_firmware and MODULE_FIRMWARE for a specific firmware > blob. Here's my proposal. > > - Before merging a patch adding or changing a firmware blob in dinq, > said blob must be publicly available at a known location, and no known > blockers for linux-firmware.git merge may exist. > > - We take care to get the blob to linux-firmware.git before the changes > hit Linus' -rc1 after the merge window. With our -rc5 feature > deadline, this should give us at least 4-5 weeks to get the blob > merged to linux-firmware.git. > > - It follows that linux-next and drm-tip may contain MODULE_FIRMWARE for > blobs that don't exist in linux-firmware.git. Agreed. And we should document this somewhere. > > BR, > Jani. > > -- > Jani Nikula, Intel Open Source Graphics Center
Quoting Rodrigo Vivi (2018-09-06 20:35:19) > On Thu, Sep 06, 2018 at 09:46:13AM +0300, Jani Nikula wrote: > > On Wed, 05 Sep 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > > > On Wed, Sep 05, 2018 at 12:07:43PM +0300, Joonas Lahtinen wrote: > > >> Was not the decision that we only gate the MODULE_FIRMWARE line until > > >> the firmware is in linux-firmware.git? > > >> > > >> So it should be no harm to support loading firmwares that are available > > >> from drm-firmware.git as long as we don't add the MODULE_FIRMWARE which > > >> would trigger false positives for distro packagers. > > > > > > As far as I can remember we had agreed on changing this and adding > > > MODULE_FIRMWARE from the beginning. > > > > > > 1. the process gets complex > > > 2. we forgot many times to add it afterwards > > > 3. module_firmware changes nothing... only the fact that initrd generation > > > wont complain if firmware is not there yet. > > > 4. The old issue was that patches were merged without the pull request > > > being sent... We fixed that by only accepting patches after pull request > > > is sent. > > > 5. By the time that all lands to distros linux-firmware.git will have > > > the firmware because of 4. > > > 6. We have now drm-firmware to mirror what official linux-firmware.git will > > > be in few weeks from now. > > > > > > So I don't see a reason anymore why to keep with complicated process with > > > split MODULE_FIRMWARE. > > > > I've changed my mind, I agree with *not* splitting MODULE_FIRMWARE > > changes to another patch anymore. > > > > However, with that I think we need to redefine when we can add > > request_firmware and MODULE_FIRMWARE for a specific firmware > > blob. Here's my proposal. > > > > - Before merging a patch adding or changing a firmware blob in dinq, > > said blob must be publicly available at a known location, and no known > > blockers for linux-firmware.git merge may exist. > > > > - We take care to get the blob to linux-firmware.git before the changes > > hit Linus' -rc1 after the merge window. With our -rc5 feature > > deadline, this should give us at least 4-5 weeks to get the blob > > merged to linux-firmware.git. > > > > - It follows that linux-next and drm-tip may contain MODULE_FIRMWARE for > > blobs that don't exist in linux-firmware.git. > > Agreed. And we should document this somewhere. Well, there are still quite a few conditionals included, if you ask me. But if you see the MODULE_FIRMWARE as a separate patch as such a burden that we should make the assumptions, I can live with it. At least I know who to call in case there was one too many conditional and we get a distro complaint again :) Regards, Joonas > > > > > BR, > > Jani. > > > > -- > > Jani Nikula, Intel Open Source Graphics Center
>-----Original Message----- >From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of >Joonas Lahtinen >Sent: Friday, September 7, 2018 12:47 AM >To: Nikula, Jani <jani.nikula@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com> >Cc: intel-gfx@lists.freedesktop.org; Zanoni, Paulo R <paulo.r.zanoni@intel.com> >Subject: Re: [Intel-gfx] [PATCH 1/1] firmware/dmc/icl: load v1.07 on icelake. > >Quoting Rodrigo Vivi (2018-09-06 20:35:19) >> On Thu, Sep 06, 2018 at 09:46:13AM +0300, Jani Nikula wrote: >> > On Wed, 05 Sep 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: >> > > On Wed, Sep 05, 2018 at 12:07:43PM +0300, Joonas Lahtinen wrote: >> > >> Was not the decision that we only gate the MODULE_FIRMWARE line >> > >> until the firmware is in linux-firmware.git? >> > >> >> > >> So it should be no harm to support loading firmwares that are >> > >> available from drm-firmware.git as long as we don't add the >> > >> MODULE_FIRMWARE which would trigger false positives for distro >packagers. >> > > >> > > As far as I can remember we had agreed on changing this and adding >> > > MODULE_FIRMWARE from the beginning. >> > > >> > > 1. the process gets complex >> > > 2. we forgot many times to add it afterwards 3. module_firmware >> > > changes nothing... only the fact that initrd generation >> > > wont complain if firmware is not there yet. >> > > 4. The old issue was that patches were merged without the pull request >> > > being sent... We fixed that by only accepting patches after pull request >> > > is sent. >> > > 5. By the time that all lands to distros linux-firmware.git will >> > > have the firmware because of 4. >> > > 6. We have now drm-firmware to mirror what official >> > > linux-firmware.git will be in few weeks from now. >> > > >> > > So I don't see a reason anymore why to keep with complicated >> > > process with split MODULE_FIRMWARE. >> > >> > I've changed my mind, I agree with *not* splitting MODULE_FIRMWARE >> > changes to another patch anymore. >> > >> > However, with that I think we need to redefine when we can add >> > request_firmware and MODULE_FIRMWARE for a specific firmware blob. >> > Here's my proposal. >> > >> > - Before merging a patch adding or changing a firmware blob in dinq, >> > said blob must be publicly available at a known location, and no known >> > blockers for linux-firmware.git merge may exist. >> > >> > - We take care to get the blob to linux-firmware.git before the changes >> > hit Linus' -rc1 after the merge window. With our -rc5 feature >> > deadline, this should give us at least 4-5 weeks to get the blob >> > merged to linux-firmware.git. >> > >> > - It follows that linux-next and drm-tip may contain MODULE_FIRMWARE for >> > blobs that don't exist in linux-firmware.git. >> >> Agreed. And we should document this somewhere. > >Well, there are still quite a few conditionals included, if you ask me. > >But if you see the MODULE_FIRMWARE as a separate patch as such a burden that >we should make the assumptions, I can live with it. At least I know who to call in >case there was one too many conditional and we get a distro complaint again :) Lets hope we wont have anyone complaining. Regarding documenting this, Rodrigo, is wiki the place for it? With regard to this patch in particular, shall I send a new version of ICL patch with MODULE_FIRMWARE in it? Thoughts? Thanks, Anusha >Regards, Joonas > >> >> > >> > BR, >> > Jani. >> > >> > -- >> > Jani Nikula, Intel Open Source Graphics Center >_______________________________________________ >Intel-gfx mailing list >Intel-gfx@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Sep 07, 2018 at 06:26:32PM +0000, Srivatsa, Anusha wrote: > > > >-----Original Message----- > >From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of > >Joonas Lahtinen > >Sent: Friday, September 7, 2018 12:47 AM > >To: Nikula, Jani <jani.nikula@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com> > >Cc: intel-gfx@lists.freedesktop.org; Zanoni, Paulo R <paulo.r.zanoni@intel.com> > >Subject: Re: [Intel-gfx] [PATCH 1/1] firmware/dmc/icl: load v1.07 on icelake. > > > >Quoting Rodrigo Vivi (2018-09-06 20:35:19) > >> On Thu, Sep 06, 2018 at 09:46:13AM +0300, Jani Nikula wrote: > >> > On Wed, 05 Sep 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > >> > > On Wed, Sep 05, 2018 at 12:07:43PM +0300, Joonas Lahtinen wrote: > >> > >> Was not the decision that we only gate the MODULE_FIRMWARE line > >> > >> until the firmware is in linux-firmware.git? > >> > >> > >> > >> So it should be no harm to support loading firmwares that are > >> > >> available from drm-firmware.git as long as we don't add the > >> > >> MODULE_FIRMWARE which would trigger false positives for distro > >packagers. > >> > > > >> > > As far as I can remember we had agreed on changing this and adding > >> > > MODULE_FIRMWARE from the beginning. > >> > > > >> > > 1. the process gets complex > >> > > 2. we forgot many times to add it afterwards 3. module_firmware > >> > > changes nothing... only the fact that initrd generation > >> > > wont complain if firmware is not there yet. > >> > > 4. The old issue was that patches were merged without the pull request > >> > > being sent... We fixed that by only accepting patches after pull request > >> > > is sent. > >> > > 5. By the time that all lands to distros linux-firmware.git will > >> > > have the firmware because of 4. > >> > > 6. We have now drm-firmware to mirror what official > >> > > linux-firmware.git will be in few weeks from now. > >> > > > >> > > So I don't see a reason anymore why to keep with complicated > >> > > process with split MODULE_FIRMWARE. > >> > > >> > I've changed my mind, I agree with *not* splitting MODULE_FIRMWARE > >> > changes to another patch anymore. > >> > > >> > However, with that I think we need to redefine when we can add > >> > request_firmware and MODULE_FIRMWARE for a specific firmware blob. > >> > Here's my proposal. > >> > > >> > - Before merging a patch adding or changing a firmware blob in dinq, > >> > said blob must be publicly available at a known location, and no known > >> > blockers for linux-firmware.git merge may exist. > >> > > >> > - We take care to get the blob to linux-firmware.git before the changes > >> > hit Linus' -rc1 after the merge window. With our -rc5 feature > >> > deadline, this should give us at least 4-5 weeks to get the blob > >> > merged to linux-firmware.git. > >> > > >> > - It follows that linux-next and drm-tip may contain MODULE_FIRMWARE for > >> > blobs that don't exist in linux-firmware.git. > >> > >> Agreed. And we should document this somewhere. > > > >Well, there are still quite a few conditionals included, if you ask me. > > > >But if you see the MODULE_FIRMWARE as a separate patch as such a burden that > >we should make the assumptions, I can live with it. At least I know who to call in > >case there was one too many conditional and we get a distro complaint again :) > > Lets hope we wont have anyone complaining. > > Regarding documenting this, Rodrigo, is wiki the place for it? I'm not sure where is the best place for this currently. So for now maybe use the internal one... Unless someone has a better idea. > With regard to this patch in particular, shall I send a new version of ICL patch with MODULE_FIRMWARE in it? > Thoughts? Let's do this to move with dmc on ICL, but for future fw patches let's do all in one patch ;) I just pushed this patch to dinq. Thanks for patch and review. Now please send a separated one for MODULE_FIRMWARE. Thanks, Rodrigo. > > Thanks, > Anusha > >Regards, Joonas > > > >> > >> > > >> > BR, > >> > Jani. > >> > > >> > -- > >> > Jani Nikula, Intel Open Source Graphics Center > >_______________________________________________ > >Intel-gfx mailing list > >Intel-gfx@lists.freedesktop.org > >https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index 1ec4f09c61f6..6d9d47322405 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -34,6 +34,9 @@ * low-power state and comes back to normal. */ +#define I915_CSR_ICL "i915/icl_dmc_ver1_07.bin" +#define ICL_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) + #define I915_CSR_GLK "i915/glk_dmc_ver1_04.bin" MODULE_FIRMWARE(I915_CSR_GLK); #define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 4) @@ -301,6 +304,8 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, if (csr->fw_path == i915_modparams.dmc_firmware_path) { /* Bypass version check for firmware override. */ required_version = csr->version; + } else if (IS_ICELAKE(dev_priv)) { + required_version = ICL_CSR_VERSION_REQUIRED; } else if (IS_CANNONLAKE(dev_priv)) { required_version = CNL_CSR_VERSION_REQUIRED; } else if (IS_GEMINILAKE(dev_priv)) { @@ -458,6 +463,8 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) if (i915_modparams.dmc_firmware_path) csr->fw_path = i915_modparams.dmc_firmware_path; + else if (IS_ICELAKE(dev_priv)) + csr->fw_path = I915_CSR_ICL; else if (IS_CANNONLAKE(dev_priv)) csr->fw_path = I915_CSR_CNL; else if (IS_GEMINILAKE(dev_priv)) diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 2852395125cd..bd7da068e813 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -3563,6 +3563,9 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv, /* 7. Setup MBUS. */ icl_mbus_init(dev_priv); + + if (resume && dev_priv->csr.dmc_payload) + intel_csr_load_program(dev_priv); } static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
Add Support to load DMC on Icelake. While at it, also add support to load the firmware during system resume. v2: load firmware during system resume.(Imre) v3: enable has_csr for icelake.(Jyoti) v4: Only load the firmware in this patch Cc: Jyoti Yadav <jyoti.r.yadav@intel.com> Cc: Imre Deak <imre.deak@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> --- drivers/gpu/drm/i915/intel_csr.c | 7 +++++++ drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++ 2 files changed, 10 insertions(+)