Re: [PATCH 1/1] firmware/dmc/icl: load v1.07 on icelake.

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx