| Message ID | 1533168445-15207-2-git-send-email-anusha.srivatsa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | firmware/dmc/icl: load v1.07 on icelake. | expand |
Em Qua, 2018-08-01 às 17:07 -0700, Anusha Srivatsa escreveu: > 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) Just to make it clear: did we test this on actual machines before submitting or are we entirely relying on the CI results? I'm not sure the CI is running enough tests to validate this patch with confidence, we'll probably need to do some manual testing here. > > 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(+) > > diff --git a/drivers/gpu/drm/i915/intel_csr.c > b/drivers/gpu/drm/i915/intel_csr.c > index cf9b600..393d419 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 cf89141..77c0986 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -3372,6 +3372,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)
On Wed, Aug 01, 2018 at 05:30:49PM -0700, Paulo Zanoni wrote: > Em Qua, 2018-08-01 às 17:07 -0700, Anusha Srivatsa escreveu: > > 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) > > Just to make it clear: did we test this on actual machines before > submitting or are we entirely relying on the CI results? > > I'm not sure the CI is running enough tests to validate this patch with > confidence, we'll probably need to do some manual testing here. At some point I believe it was agreed that CI would test this and get the new firmware automatically from the cover-letter. The problem is that I don't see any cover-letter so I'm afraid it is not running with the new firmware. Tomi? Also I believe in case it has the cover letter it should run the full CI on the machine or at least stash it and run on the weekend or whenever we run the full on all machines and then report back again. Possible? Martin? > > > > > 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(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c > > b/drivers/gpu/drm/i915/intel_csr.c > > index cf9b600..393d419 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 cf89141..77c0986 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -3372,6 +3372,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) > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi Anusha, I think we should also add "HAS_CSR" capability, which is being exercised inside intel_csr_ucode_init() path. For ICL, inside intel_device_info structure we should add has_csr = 1, otherwise below check will fail and function will return from there itself. if (!HAS_CSR(dev_priv)) return; Regards Jyoti On 8/2/2018 10:41 AM, Rodrigo Vivi wrote: > On Wed, Aug 01, 2018 at 05:30:49PM -0700, Paulo Zanoni wrote: >> Em Qua, 2018-08-01 às 17:07 -0700, Anusha Srivatsa escreveu: >>> 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) >> Just to make it clear: did we test this on actual machines before >> submitting or are we entirely relying on the CI results? >> >> I'm not sure the CI is running enough tests to validate this patch with >> confidence, we'll probably need to do some manual testing here. > At some point I believe it was agreed that CI would test this > and get the new firmware automatically from the cover-letter. > > The problem is that I don't see any cover-letter so I'm afraid > it is not running with the new firmware. > > Tomi? > > Also I believe in case it has the cover letter it should run > the full CI on the machine or at least stash it and run on > the weekend or whenever we run the full on all machines and > then report back again. Possible? > > Martin? > >>> 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(+) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_csr.c >>> b/drivers/gpu/drm/i915/intel_csr.c >>> index cf9b600..393d419 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 cf89141..77c0986 100644 >>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c >>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c >>> @@ -3372,6 +3372,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) >> _______________________________________________ >> 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 08/02/2018 08:11 AM, Rodrigo Vivi wrote: > On Wed, Aug 01, 2018 at 05:30:49PM -0700, Paulo Zanoni wrote: >> Em Qua, 2018-08-01 às 17:07 -0700, Anusha Srivatsa escreveu: >>> 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) >> >> Just to make it clear: did we test this on actual machines before >> submitting or are we entirely relying on the CI results? >> >> I'm not sure the CI is running enough tests to validate this patch with >> confidence, we'll probably need to do some manual testing here. > > At some point I believe it was agreed that CI would test this > and get the new firmware automatically from the cover-letter. > > The problem is that I don't see any cover-letter so I'm afraid > it is not running with the new firmware. > > Tomi? The requests can be checked from patchwork REST API: https://patchwork.freedesktop.org/api/1.0/projects/intel-gfx/events/?page=1&name=pull-request-new There has been a pull request for new firmware, but it couldn't be acted. CI can't connect to SSH repository, because those generally need an account. This has been tried and tested before. Better way is to use git://, http:// or https:// URLs, as the pull shouldn't need any special permissions. This time I have manually pulled git://anongit.freedesktop.org/drm/drm-firmware/master but, if you want to make this automatic, the pull requests shouldn't be from repositories with obligatory login. Another note: known firmware pull repositories are now freedesktop.org git://anongit.freedesktop.org/drm/drm-firmware (fetch) g_anushasr git://github.com/anushasr/linux-firmware.git (fetch) h_anushasr https://github.com/anushasr/linux-firmware.git (fetch) kernel.org git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware (fetch) If you know that in future there might be pull requests from another repository, please inform me about that in advance. Tomi > > Also I believe in case it has the cover letter it should run > the full CI on the machine or at least stash it and run on > the weekend or whenever we run the full on all machines and > then report back again. Possible? > > Martin? > >> >>> >>> 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(+) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_csr.c >>> b/drivers/gpu/drm/i915/intel_csr.c >>> index cf9b600..393d419 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 cf89141..77c0986 100644 >>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c >>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c >>> @@ -3372,6 +3372,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) >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx Tomi
On 08/02/2018 10:15 AM, Tomi Sarvela wrote: > On 08/02/2018 08:11 AM, Rodrigo Vivi wrote: >> On Wed, Aug 01, 2018 at 05:30:49PM -0700, Paulo Zanoni wrote: >>> Em Qua, 2018-08-01 às 17:07 -0700, Anusha Srivatsa escreveu: >>>> 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) >>> >>> Just to make it clear: did we test this on actual machines before >>> submitting or are we entirely relying on the CI results? >>> >>> I'm not sure the CI is running enough tests to validate this patch with >>> confidence, we'll probably need to do some manual testing here. >> >> At some point I believe it was agreed that CI would test this >> and get the new firmware automatically from the cover-letter. >> >> The problem is that I don't see any cover-letter so I'm afraid >> it is not running with the new firmware. >> >> Tomi? > > The requests can be checked from patchwork REST API: > > https://patchwork.freedesktop.org/api/1.0/projects/intel-gfx/events/?page=1&name=pull-request-new > > > There has been a pull request for new firmware, but it couldn't be > acted. CI can't connect to SSH repository, because those generally need > an account. This has been tried and tested before. Better way is to use > git://, http:// or https:// URLs, as the pull shouldn't need any special > permissions. > > This time I have manually pulled > git://anongit.freedesktop.org/drm/drm-firmware/master but, if you want > to make this automatic, the pull requests shouldn't be from repositories > with obligatory login. > > Another note: known firmware pull repositories are now > > freedesktop.org git://anongit.freedesktop.org/drm/drm-firmware (fetch) > g_anushasr git://github.com/anushasr/linux-firmware.git (fetch) > h_anushasr https://github.com/anushasr/linux-firmware.git (fetch) > kernel.org > git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware > (fetch) > > If you know that in future there might be pull requests from another > repository, please inform me about that in advance. The results from the re-tested patchset are back, with dmesgs: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9839/fi-icl-u/boot0.log https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9839/fi-icl-u/dmesg0.log Testrun hung in igt@drv_selftest@live_hangcheck, which is standard place for ICL to rest in peace. https://intel-gfx-ci.01.org/tree/drm-tip/fi-icl-u.html Tomi > >> >> Also I believe in case it has the cover letter it should run >> the full CI on the machine or at least stash it and run on >> the weekend or whenever we run the full on all machines and >> then report back again. Possible? >> >> Martin? >> >>> >>>> >>>> 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(+) >>>> >>>> diff --git a/drivers/gpu/drm/i915/intel_csr.c >>>> b/drivers/gpu/drm/i915/intel_csr.c >>>> index cf9b600..393d419 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 cf89141..77c0986 100644 >>>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c >>>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c >>>> @@ -3372,6 +3372,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) >>> _______________________________________________ >>> Intel-gfx mailing list >>> Intel-gfx@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > Tomi Tomi
>-----Original Message----- >From: Sarvela, Tomi P >Sent: Thursday, August 2, 2018 12:51 AM >To: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Zanoni, Paulo R ><paulo.r.zanoni@intel.com> >Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel- >gfx@lists.freedesktop.org; Martin Peres <martin.peres@linux.intel.com> >Subject: Re: [Intel-gfx] [PATCH] firmware/dmc/icl: load v1.07 on icelake. > >On 08/02/2018 10:15 AM, Tomi Sarvela wrote: >> On 08/02/2018 08:11 AM, Rodrigo Vivi wrote: >>> On Wed, Aug 01, 2018 at 05:30:49PM -0700, Paulo Zanoni wrote: >>>> Em Qua, 2018-08-01 às 17:07 -0700, Anusha Srivatsa escreveu: >>>>> 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) >>>> >>>> Just to make it clear: did we test this on actual machines before >>>> submitting or are we entirely relying on the CI results? >>>> >>>> I'm not sure the CI is running enough tests to validate this patch >>>> with confidence, we'll probably need to do some manual testing here. >>> >>> At some point I believe it was agreed that CI would test this and get >>> the new firmware automatically from the cover-letter. >>> >>> The problem is that I don't see any cover-letter so I'm afraid it is >>> not running with the new firmware. >>> >>> Tomi? >> >> The requests can be checked from patchwork REST API: >> >> https://patchwork.freedesktop.org/api/1.0/projects/intel-gfx/events/?p >> age=1&name=pull-request-new >> >> >> There has been a pull request for new firmware, but it couldn't be >> acted. CI can't connect to SSH repository, because those generally >> need an account. This has been tried and tested before. Better way is >> to use git://, http:// or https:// URLs, as the pull shouldn't need >> any special permissions. >> >> This time I have manually pulled >> git://anongit.freedesktop.org/drm/drm-firmware/master but, if you want >> to make this automatic, the pull requests shouldn't be from >> repositories with obligatory login. >> >> Another note: known firmware pull repositories are now >> >> freedesktop.org git://anongit.freedesktop.org/drm/drm-firmware >> (fetch) g_anushasr git://github.com/anushasr/linux-firmware.git >> (fetch) h_anushasr https://github.com/anushasr/linux-firmware.git >> (fetch) kernel.org >> git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware >> (fetch) >> >> If you know that in future there might be pull requests from another >> repository, please inform me about that in advance. > >The results from the re-tested patchset are back, with dmesgs: > >https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9839/fi-icl-u/boot0.log > >https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9839/fi-icl-u/dmesg0.log > >Testrun hung in igt@drv_selftest@live_hangcheck, which is standard place for ICL >to rest in peace. > >https://intel-gfx-ci.01.org/tree/drm-tip/fi-icl-u.html > >Tomi > > > >> >>> >>> Also I believe in case it has the cover letter it should run the full >>> CI on the machine or at least stash it and run on the weekend or >>> whenever we run the full on all machines and then report back again. >>> Possible? >>> Martin? For Firmware patches, it would be nice to have the whole IGT running with the patch ofcourse. Martin, Tomi is this possible? Anusha >>>> >>>>> >>>>> 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(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/i915/intel_csr.c >>>>> b/drivers/gpu/drm/i915/intel_csr.c >>>>> index cf9b600..393d419 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 cf89141..77c0986 100644 >>>>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c >>>>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c >>>>> @@ -3372,6 +3372,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) >>>> _______________________________________________ >>>> Intel-gfx mailing list >>>> Intel-gfx@lists.freedesktop.org >>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >> >> Tomi > > >Tomi >-- >Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
>-----Original Message----- >From: Yadav, Jyoti R >Sent: Wednesday, August 1, 2018 10:34 PM >To: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel- >gfx@lists.freedesktop.org; Zanoni, Paulo R <paulo.r.zanoni@intel.com> >Cc: Sarvela, Tomi P <tomi.p.sarvela@intel.com>; Vivi, Rodrigo ><rodrigo.vivi@intel.com>; Zanoni, Paulo R <paulo.r.zanoni@intel.com>; intel- >gfx@lists.freedesktop.org >Subject: Re: [Intel-gfx] [PATCH] firmware/dmc/icl: load v1.07 on icelake. > >Hi Anusha, > >I think we should also add "HAS_CSR" capability, which is being exercised inside >intel_csr_ucode_init() path. Yes you are right :) Thanks for pointing it. >For ICL, inside intel_device_info structure we should add has_csr = 1, otherwise >below check will fail and function will return from there itself. > >if (!HAS_CSR(dev_priv)) > return; That is what is happening according to the logs. Rodrigo, Paulo : The has_csr = 1 should be a separate patch though. This patch should just try and load the blob and has_csr patch has to enable csr on ICL. Anusha >Regards >Jyoti >On 8/2/2018 10:41 AM, Rodrigo Vivi wrote: >> On Wed, Aug 01, 2018 at 05:30:49PM -0700, Paulo Zanoni wrote: >>> Em Qua, 2018-08-01 às 17:07 -0700, Anusha Srivatsa escreveu: >>>> 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) >>> Just to make it clear: did we test this on actual machines before >>> submitting or are we entirely relying on the CI results? >>> >>> I'm not sure the CI is running enough tests to validate this patch >>> with confidence, we'll probably need to do some manual testing here. >> At some point I believe it was agreed that CI would test this and get >> the new firmware automatically from the cover-letter. >> >> The problem is that I don't see any cover-letter so I'm afraid it is >> not running with the new firmware. >> >> Tomi? >> >> Also I believe in case it has the cover letter it should run the full >> CI on the machine or at least stash it and run on the weekend or >> whenever we run the full on all machines and then report back again. >> Possible? >> >> Martin? >> >>>> 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(+) >>>> >>>> diff --git a/drivers/gpu/drm/i915/intel_csr.c >>>> b/drivers/gpu/drm/i915/intel_csr.c >>>> index cf9b600..393d419 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 cf89141..77c0986 100644 >>>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c >>>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c >>>> @@ -3372,6 +3372,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) >>> _______________________________________________ >>> 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 cf9b600..393d419 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 cf89141..77c0986 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -3372,6 +3372,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) 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(+)