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

Wed Sep 5 19:38:49 UTC 2018

>-----Original Message-----
>From: Vivi, Rodrigo
>Sent: Wednesday, September 5, 2018 12:31 PM
>To: Srivatsa, Anusha <anusha.srivatsa at intel.com>
>Cc: Deak, Imre <imre.deak at intel.com>; Nikula, Jani <jani.nikula at intel.com>;
>intel-gfx at lists.freedesktop.org; Zanoni, Paulo R <paulo.r.zanoni at 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 at intel.com>
>> >Cc: Srivatsa, Anusha <anusha.srivatsa at intel.com>; Nikula, Jani
>> ><jani.nikula at intel.com>; intel-gfx at lists.freedesktop.org; Zanoni,
>> >Paulo R <paulo.r.zanoni at 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 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.
>>
>> 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 at lists.freedesktop.org
>> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx