[Intel-gfx] [PATCH 1/1] firmware/dmc/icl: load v1.07 on icelake.

Wed Sep 5 16:42:28 UTC 2018

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 at intel.com>
> > > > Cc: Imre Deak <imre.deak at intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > > Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
> > > 
> > > Reviewed-by: Imre Deak <imre.deak at 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 at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx