* [PATCH] drm/i915: add support for specifying DMC firmware override by module param @ 2017-11-21 11:51 Jani Nikula 2017-11-21 13:43 ` David Weinehall 2017-11-21 13:48 ` ✗ Fi.CI.BAT: failure for " Patchwork 0 siblings, 2 replies; 9+ messages in thread From: Jani Nikula @ 2017-11-21 11:51 UTC (permalink / raw) To: intel-gfx; +Cc: jani.nikula Use i915.dmc_firmware_path to override default firmware for the platform and bypassing version checks. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- Untested. --- drivers/gpu/drm/i915/i915_params.c | 3 +++ drivers/gpu/drm/i915/intel_csr.c | 9 +++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 3328147b4863..c11ad6d67fa9 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -171,6 +171,9 @@ i915_param_named_unsafe(guc_firmware_path, charp, 0400, i915_param_named_unsafe(huc_firmware_path, charp, 0400, "HuC firmware path to use instead of the default one"); +i915_param_named_unsafe(dmc_firmware_path, charp, 0400, + "DMC firmware path to use instead of the default one"); + i915_param_named_unsafe(enable_dp_mst, bool, 0600, "Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)"); diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index 77d8b3d483ca..82db376ec7e1 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -296,7 +296,10 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, csr->version = css_header->version; - if (IS_CANNONLAKE(dev_priv)) { + if (csr->fw_path == i915_modparams.dmc_firmware_path) { + /* Bypass version check for firmware override. */ + required_version = csr->version; + } else if (IS_CANNONLAKE(dev_priv)) { required_version = CNL_CSR_VERSION_REQUIRED; } else if (IS_GEMINILAKE(dev_priv)) { required_version = GLK_CSR_VERSION_REQUIRED; @@ -451,7 +454,9 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) if (!HAS_CSR(dev_priv)) return; - if (IS_CANNONLAKE(dev_priv)) + if (i915_modparams.dmc_firmware_path) + csr->fw_path = i915_modparams.dmc_firmware_path; + else if (IS_CANNONLAKE(dev_priv)) csr->fw_path = I915_CSR_CNL; else if (IS_GEMINILAKE(dev_priv)) csr->fw_path = I915_CSR_GLK; -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: add support for specifying DMC firmware override by module param 2017-11-21 11:51 [PATCH] drm/i915: add support for specifying DMC firmware override by module param Jani Nikula @ 2017-11-21 13:43 ` David Weinehall 2017-11-21 21:54 ` Jani Nikula 2017-11-21 13:48 ` ✗ Fi.CI.BAT: failure for " Patchwork 1 sibling, 1 reply; 9+ messages in thread From: David Weinehall @ 2017-11-21 13:43 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx On Tue, Nov 21, 2017 at 01:51:29PM +0200, Jani Nikula wrote: > Use i915.dmc_firmware_path to override default firmware for the platform > and bypassing version checks. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > --- > > Untested. Yeah, it kind of shows. It fails to compile :D But if you chuck in: param(char *, dmc_firmware_path, NULL) \ in i915_params.h Things work correctly and you can use: Tested-by: David Weinehall <david.weinehall@linux.intel.com> Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_params.c | 3 +++ > drivers/gpu/drm/i915/intel_csr.c | 9 +++++++-- > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > index 3328147b4863..c11ad6d67fa9 100644 > --- a/drivers/gpu/drm/i915/i915_params.c > +++ b/drivers/gpu/drm/i915/i915_params.c > @@ -171,6 +171,9 @@ i915_param_named_unsafe(guc_firmware_path, charp, 0400, > i915_param_named_unsafe(huc_firmware_path, charp, 0400, > "HuC firmware path to use instead of the default one"); > > +i915_param_named_unsafe(dmc_firmware_path, charp, 0400, > + "DMC firmware path to use instead of the default one"); > + > i915_param_named_unsafe(enable_dp_mst, bool, 0600, > "Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)"); > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > index 77d8b3d483ca..82db376ec7e1 100644 > --- a/drivers/gpu/drm/i915/intel_csr.c > +++ b/drivers/gpu/drm/i915/intel_csr.c > @@ -296,7 +296,10 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, > > csr->version = css_header->version; > > - if (IS_CANNONLAKE(dev_priv)) { > + if (csr->fw_path == i915_modparams.dmc_firmware_path) { > + /* Bypass version check for firmware override. */ > + required_version = csr->version; > + } else if (IS_CANNONLAKE(dev_priv)) { > required_version = CNL_CSR_VERSION_REQUIRED; > } else if (IS_GEMINILAKE(dev_priv)) { > required_version = GLK_CSR_VERSION_REQUIRED; > @@ -451,7 +454,9 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) > if (!HAS_CSR(dev_priv)) > return; > > - if (IS_CANNONLAKE(dev_priv)) > + if (i915_modparams.dmc_firmware_path) > + csr->fw_path = i915_modparams.dmc_firmware_path; > + else if (IS_CANNONLAKE(dev_priv)) > csr->fw_path = I915_CSR_CNL; > else if (IS_GEMINILAKE(dev_priv)) > csr->fw_path = I915_CSR_GLK; > -- > 2.11.0 > > _______________________________________________ > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: add support for specifying DMC firmware override by module param 2017-11-21 13:43 ` David Weinehall @ 2017-11-21 21:54 ` Jani Nikula 2017-11-22 8:05 ` Joonas Lahtinen 2017-11-22 16:06 ` David Weinehall 0 siblings, 2 replies; 9+ messages in thread From: Jani Nikula @ 2017-11-21 21:54 UTC (permalink / raw) To: David Weinehall; +Cc: intel-gfx On Tue, 21 Nov 2017, David Weinehall <david.weinehall@linux.intel.com> wrote: > On Tue, Nov 21, 2017 at 01:51:29PM +0200, Jani Nikula wrote: >> Use i915.dmc_firmware_path to override default firmware for the platform >> and bypassing version checks. >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> >> --- >> >> Untested. > > Yeah, it kind of shows. It fails to compile :D Oops. I was anxious to get the patch on the list in the heat of IRC discussion, and just added the disclaimer instead of doing this properly... *blush* > > But if you chuck in: > > param(char *, dmc_firmware_path, NULL) \ > > in i915_params.h > > Things work correctly and you can use: > > Tested-by: David Weinehall <david.weinehall@linux.intel.com> > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> Thanks. But let's wait for more input on whether this is really what people want. Perhaps this is useful *regardless* of the outcome of the other discussions. BR, Jani. > >> --- >> drivers/gpu/drm/i915/i915_params.c | 3 +++ >> drivers/gpu/drm/i915/intel_csr.c | 9 +++++++-- >> 2 files changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c >> index 3328147b4863..c11ad6d67fa9 100644 >> --- a/drivers/gpu/drm/i915/i915_params.c >> +++ b/drivers/gpu/drm/i915/i915_params.c >> @@ -171,6 +171,9 @@ i915_param_named_unsafe(guc_firmware_path, charp, 0400, >> i915_param_named_unsafe(huc_firmware_path, charp, 0400, >> "HuC firmware path to use instead of the default one"); >> >> +i915_param_named_unsafe(dmc_firmware_path, charp, 0400, >> + "DMC firmware path to use instead of the default one"); >> + >> i915_param_named_unsafe(enable_dp_mst, bool, 0600, >> "Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)"); >> >> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c >> index 77d8b3d483ca..82db376ec7e1 100644 >> --- a/drivers/gpu/drm/i915/intel_csr.c >> +++ b/drivers/gpu/drm/i915/intel_csr.c >> @@ -296,7 +296,10 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, >> >> csr->version = css_header->version; >> >> - if (IS_CANNONLAKE(dev_priv)) { >> + if (csr->fw_path == i915_modparams.dmc_firmware_path) { >> + /* Bypass version check for firmware override. */ >> + required_version = csr->version; >> + } else if (IS_CANNONLAKE(dev_priv)) { >> required_version = CNL_CSR_VERSION_REQUIRED; >> } else if (IS_GEMINILAKE(dev_priv)) { >> required_version = GLK_CSR_VERSION_REQUIRED; >> @@ -451,7 +454,9 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) >> if (!HAS_CSR(dev_priv)) >> return; >> >> - if (IS_CANNONLAKE(dev_priv)) >> + if (i915_modparams.dmc_firmware_path) >> + csr->fw_path = i915_modparams.dmc_firmware_path; >> + else if (IS_CANNONLAKE(dev_priv)) >> csr->fw_path = I915_CSR_CNL; >> else if (IS_GEMINILAKE(dev_priv)) >> csr->fw_path = I915_CSR_GLK; >> -- >> 2.11.0 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: add support for specifying DMC firmware override by module param 2017-11-21 21:54 ` Jani Nikula @ 2017-11-22 8:05 ` Joonas Lahtinen 2017-11-22 16:20 ` Jani Nikula 2017-11-22 16:06 ` David Weinehall 1 sibling, 1 reply; 9+ messages in thread From: Joonas Lahtinen @ 2017-11-22 8:05 UTC (permalink / raw) To: Jani Nikula, David Weinehall; +Cc: intel-gfx On Tue, 2017-11-21 at 23:54 +0200, Jani Nikula wrote: > On Tue, 21 Nov 2017, David Weinehall <david.weinehall@linux.intel.com> wrote: > > On Tue, Nov 21, 2017 at 01:51:29PM +0200, Jani Nikula wrote: > > > Use i915.dmc_firmware_path to override default firmware for the platform > > > and bypassing version checks. > > > > > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > > > > > --- > > > > > > Untested. > > > > Yeah, it kind of shows. It fails to compile :D > > Oops. I was anxious to get the patch on the list in the heat of IRC > discussion, and just added the disclaimer instead of doing this > properly... *blush* > > > > > But if you chuck in: > > > > param(char *, dmc_firmware_path, NULL) \ I think I already raised my suggestion earlier, but here it goes again. Maybe we should consider consolidating these into: i915.firmware=guc:whatever.bin i915.firmware=dmc:whatever.bin Then something like "i915.firmware=guc:disable" could be supported, too. Regards, Joonas > > > > in i915_params.h > > > > Things work correctly and you can use: > > > > Tested-by: David Weinehall <david.weinehall@linux.intel.com> > > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> > > Thanks. But let's wait for more input on whether this is really what > people want. Perhaps this is useful *regardless* of the outcome of the > other discussions. > > BR, > Jani. > > > > > > > --- > > > drivers/gpu/drm/i915/i915_params.c | 3 +++ > > > drivers/gpu/drm/i915/intel_csr.c | 9 +++++++-- > > > 2 files changed, 10 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > > > index 3328147b4863..c11ad6d67fa9 100644 > > > --- a/drivers/gpu/drm/i915/i915_params.c > > > +++ b/drivers/gpu/drm/i915/i915_params.c > > > @@ -171,6 +171,9 @@ i915_param_named_unsafe(guc_firmware_path, charp, 0400, > > > i915_param_named_unsafe(huc_firmware_path, charp, 0400, > > > "HuC firmware path to use instead of the default one"); > > > > > > +i915_param_named_unsafe(dmc_firmware_path, charp, 0400, > > > + "DMC firmware path to use instead of the default one"); > > > + > > > i915_param_named_unsafe(enable_dp_mst, bool, 0600, > > > "Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)"); > > > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > > > index 77d8b3d483ca..82db376ec7e1 100644 > > > --- a/drivers/gpu/drm/i915/intel_csr.c > > > +++ b/drivers/gpu/drm/i915/intel_csr.c > > > @@ -296,7 +296,10 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, > > > > > > csr->version = css_header->version; > > > > > > - if (IS_CANNONLAKE(dev_priv)) { > > > + if (csr->fw_path == i915_modparams.dmc_firmware_path) { > > > + /* Bypass version check for firmware override. */ > > > + required_version = csr->version; > > > + } else if (IS_CANNONLAKE(dev_priv)) { > > > required_version = CNL_CSR_VERSION_REQUIRED; > > > } else if (IS_GEMINILAKE(dev_priv)) { > > > required_version = GLK_CSR_VERSION_REQUIRED; > > > @@ -451,7 +454,9 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) > > > if (!HAS_CSR(dev_priv)) > > > return; > > > > > > - if (IS_CANNONLAKE(dev_priv)) > > > + if (i915_modparams.dmc_firmware_path) > > > + csr->fw_path = i915_modparams.dmc_firmware_path; > > > + else if (IS_CANNONLAKE(dev_priv)) > > > csr->fw_path = I915_CSR_CNL; > > > else if (IS_GEMINILAKE(dev_priv)) > > > csr->fw_path = I915_CSR_GLK; > > > -- > > > 2.11.0 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: add support for specifying DMC firmware override by module param 2017-11-22 8:05 ` Joonas Lahtinen @ 2017-11-22 16:20 ` Jani Nikula 2017-12-21 18:32 ` Srivatsa, Anusha 2018-01-10 0:29 ` Srivatsa, Anusha 0 siblings, 2 replies; 9+ messages in thread From: Jani Nikula @ 2017-11-22 16:20 UTC (permalink / raw) To: Joonas Lahtinen, David Weinehall; +Cc: intel-gfx On Wed, 22 Nov 2017, Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote: > On Tue, 2017-11-21 at 23:54 +0200, Jani Nikula wrote: >> On Tue, 21 Nov 2017, David Weinehall <david.weinehall@linux.intel.com> wrote: >> > On Tue, Nov 21, 2017 at 01:51:29PM +0200, Jani Nikula wrote: >> > > Use i915.dmc_firmware_path to override default firmware for the platform >> > > and bypassing version checks. >> > > >> > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> > > >> > > --- >> > > >> > > Untested. >> > >> > Yeah, it kind of shows. It fails to compile :D >> >> Oops. I was anxious to get the patch on the list in the heat of IRC >> discussion, and just added the disclaimer instead of doing this >> properly... *blush* >> >> > >> > But if you chuck in: >> > >> > param(char *, dmc_firmware_path, NULL) \ > > I think I already raised my suggestion earlier, but here it goes again. > Maybe we should consider consolidating these into: > > i915.firmware=guc:whatever.bin i915.firmware=dmc:whatever.bin I don't think that's possible. I didn't double check but I think you only get one value from the module param code. Unless you roll your own modparam hooks, which is best avoided. > Then something like "i915.firmware=guc:disable" could be supported, > too. i915.dmc_firmware_path="" could mean disable. BR, Jani. > > Regards, Joonas > >> > >> > in i915_params.h >> > >> > Things work correctly and you can use: >> > >> > Tested-by: David Weinehall <david.weinehall@linux.intel.com> >> > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> >> >> Thanks. But let's wait for more input on whether this is really what >> people want. Perhaps this is useful *regardless* of the outcome of the >> other discussions. >> >> BR, >> Jani. >> >> >> > >> > > --- >> > > drivers/gpu/drm/i915/i915_params.c | 3 +++ >> > > drivers/gpu/drm/i915/intel_csr.c | 9 +++++++-- >> > > 2 files changed, 10 insertions(+), 2 deletions(-) >> > > >> > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c >> > > index 3328147b4863..c11ad6d67fa9 100644 >> > > --- a/drivers/gpu/drm/i915/i915_params.c >> > > +++ b/drivers/gpu/drm/i915/i915_params.c >> > > @@ -171,6 +171,9 @@ i915_param_named_unsafe(guc_firmware_path, charp, 0400, >> > > i915_param_named_unsafe(huc_firmware_path, charp, 0400, >> > > "HuC firmware path to use instead of the default one"); >> > > >> > > +i915_param_named_unsafe(dmc_firmware_path, charp, 0400, >> > > + "DMC firmware path to use instead of the default one"); >> > > + >> > > i915_param_named_unsafe(enable_dp_mst, bool, 0600, >> > > "Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)"); >> > > >> > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c >> > > index 77d8b3d483ca..82db376ec7e1 100644 >> > > --- a/drivers/gpu/drm/i915/intel_csr.c >> > > +++ b/drivers/gpu/drm/i915/intel_csr.c >> > > @@ -296,7 +296,10 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, >> > > >> > > csr->version = css_header->version; >> > > >> > > - if (IS_CANNONLAKE(dev_priv)) { >> > > + if (csr->fw_path == i915_modparams.dmc_firmware_path) { >> > > + /* Bypass version check for firmware override. */ >> > > + required_version = csr->version; >> > > + } else if (IS_CANNONLAKE(dev_priv)) { >> > > required_version = CNL_CSR_VERSION_REQUIRED; >> > > } else if (IS_GEMINILAKE(dev_priv)) { >> > > required_version = GLK_CSR_VERSION_REQUIRED; >> > > @@ -451,7 +454,9 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) >> > > if (!HAS_CSR(dev_priv)) >> > > return; >> > > >> > > - if (IS_CANNONLAKE(dev_priv)) >> > > + if (i915_modparams.dmc_firmware_path) >> > > + csr->fw_path = i915_modparams.dmc_firmware_path; >> > > + else if (IS_CANNONLAKE(dev_priv)) >> > > csr->fw_path = I915_CSR_CNL; >> > > else if (IS_GEMINILAKE(dev_priv)) >> > > csr->fw_path = I915_CSR_GLK; >> > > -- >> > > 2.11.0 >> > > >> > > _______________________________________________ >> > > Intel-gfx mailing list >> > > Intel-gfx@lists.freedesktop.org >> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >> -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: add support for specifying DMC firmware override by module param 2017-11-22 16:20 ` Jani Nikula @ 2017-12-21 18:32 ` Srivatsa, Anusha 2018-01-10 0:29 ` Srivatsa, Anusha 1 sibling, 0 replies; 9+ messages in thread From: Srivatsa, Anusha @ 2017-12-21 18:32 UTC (permalink / raw) To: Nikula, Jani, Joonas Lahtinen, David Weinehall Cc: intel-gfx@lists.freedesktop.org >-----Original Message----- >From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of >Jani Nikula >Sent: Wednesday, November 22, 2017 8:20 AM >To: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>; David Weinehall ><david.weinehall@linux.intel.com> >Cc: intel-gfx@lists.freedesktop.org >Subject: Re: [Intel-gfx] [PATCH] drm/i915: add support for specifying DMC >firmware override by module param > >On Wed, 22 Nov 2017, Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >wrote: >> On Tue, 2017-11-21 at 23:54 +0200, Jani Nikula wrote: >>> On Tue, 21 Nov 2017, David Weinehall <david.weinehall@linux.intel.com> >wrote: >>> > On Tue, Nov 21, 2017 at 01:51:29PM +0200, Jani Nikula wrote: >>> > > Use i915.dmc_firmware_path to override default firmware for the >>> > > platform and bypassing version checks. >>> > > >>> > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> >>> > > >>> > > --- >>> > > >>> > > Untested. >>> > >>> > Yeah, it kind of shows. It fails to compile :D >>> >>> Oops. I was anxious to get the patch on the list in the heat of IRC >>> discussion, and just added the disclaimer instead of doing this >>> properly... *blush* >>> >>> > >>> > But if you chuck in: >>> > >>> > param(char *, dmc_firmware_path, NULL) \ >> >> I think I already raised my suggestion earlier, but here it goes again. >> Maybe we should consider consolidating these into: >> >> i915.firmware=guc:whatever.bin i915.firmware=dmc:whatever.bin > >I don't think that's possible. I didn't double check but I think you only get one >value from the module param code. Unless you roll your own modparam hooks, >which is best avoided. > >> Then something like "i915.firmware=guc:disable" could be supported, >> too. > >i915.dmc_firmware_path="" could mean disable. > >BR, >Jani. > > >> >> Regards, Joonas >> >>> > >>> > in i915_params.h >>> > >>> > Things work correctly and you can use: >>> > >>> > Tested-by: David Weinehall <david.weinehall@linux.intel.com> >>> > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> >>> >>> Thanks. But let's wait for more input on whether this is really what >>> people want. Perhaps this is useful *regardless* of the outcome of >>> the other discussions. I believe this patch is going to be useful anyway.... IT would make things a lot easier for testing purpose. I think we should get this merged. Does anyone have any concerns? Anusha >>> BR, >>> Jani. >>> >>> >>> > >>> > > --- >>> > > drivers/gpu/drm/i915/i915_params.c | 3 +++ >>> > > drivers/gpu/drm/i915/intel_csr.c | 9 +++++++-- >>> > > 2 files changed, 10 insertions(+), 2 deletions(-) >>> > > >>> > > diff --git a/drivers/gpu/drm/i915/i915_params.c >>> > > b/drivers/gpu/drm/i915/i915_params.c >>> > > index 3328147b4863..c11ad6d67fa9 100644 >>> > > --- a/drivers/gpu/drm/i915/i915_params.c >>> > > +++ b/drivers/gpu/drm/i915/i915_params.c >>> > > @@ -171,6 +171,9 @@ i915_param_named_unsafe(guc_firmware_path, >>> > > charp, 0400, i915_param_named_unsafe(huc_firmware_path, charp, >0400, >>> > > "HuC firmware path to use instead of the default one"); >>> > > >>> > > +i915_param_named_unsafe(dmc_firmware_path, charp, 0400, >>> > > + "DMC firmware path to use instead of the default one"); >>> > > + >>> > > i915_param_named_unsafe(enable_dp_mst, bool, 0600, >>> > > "Enable multi-stream transport (MST) for new DisplayPort sinks. >>> > > (default: true)"); >>> > > >>> > > diff --git a/drivers/gpu/drm/i915/intel_csr.c >>> > > b/drivers/gpu/drm/i915/intel_csr.c >>> > > index 77d8b3d483ca..82db376ec7e1 100644 >>> > > --- a/drivers/gpu/drm/i915/intel_csr.c >>> > > +++ b/drivers/gpu/drm/i915/intel_csr.c >>> > > @@ -296,7 +296,10 @@ static uint32_t *parse_csr_fw(struct >>> > > drm_i915_private *dev_priv, >>> > > >>> > > csr->version = css_header->version; >>> > > >>> > > - if (IS_CANNONLAKE(dev_priv)) { >>> > > + if (csr->fw_path == i915_modparams.dmc_firmware_path) { >>> > > + /* Bypass version check for firmware override. */ >>> > > + required_version = csr->version; >>> > > + } else if (IS_CANNONLAKE(dev_priv)) { >>> > > required_version = CNL_CSR_VERSION_REQUIRED; >>> > > } else if (IS_GEMINILAKE(dev_priv)) { >>> > > required_version = GLK_CSR_VERSION_REQUIRED; @@ -451,7 >+454,9 >>> > > @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) >>> > > if (!HAS_CSR(dev_priv)) >>> > > return; >>> > > >>> > > - if (IS_CANNONLAKE(dev_priv)) >>> > > + if (i915_modparams.dmc_firmware_path) >>> > > + csr->fw_path = i915_modparams.dmc_firmware_path; >>> > > + else if (IS_CANNONLAKE(dev_priv)) >>> > > csr->fw_path = I915_CSR_CNL; >>> > > else if (IS_GEMINILAKE(dev_priv)) >>> > > csr->fw_path = I915_CSR_GLK; >>> > > -- >>> > > 2.11.0 >>> > > >>> > > _______________________________________________ >>> > > Intel-gfx mailing list >>> > > Intel-gfx@lists.freedesktop.org >>> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >>> >>> > >-- >Jani Nikula, Intel Open Source Technology 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: add support for specifying DMC firmware override by module param 2017-11-22 16:20 ` Jani Nikula 2017-12-21 18:32 ` Srivatsa, Anusha @ 2018-01-10 0:29 ` Srivatsa, Anusha 1 sibling, 0 replies; 9+ messages in thread From: Srivatsa, Anusha @ 2018-01-10 0:29 UTC (permalink / raw) To: Nikula, Jani, Joonas Lahtinen, David Weinehall Cc: intel-gfx@lists.freedesktop.org Hi, This patch could be really helpful.... Is there any issue with it? It's not merged yet..... Regards, Anusha >-----Original Message----- >From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of >Jani Nikula >Sent: Wednesday, November 22, 2017 8:20 AM >To: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>; David Weinehall ><david.weinehall@linux.intel.com> >Cc: intel-gfx@lists.freedesktop.org >Subject: Re: [Intel-gfx] [PATCH] drm/i915: add support for specifying DMC >firmware override by module param > >On Wed, 22 Nov 2017, Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >wrote: >> On Tue, 2017-11-21 at 23:54 +0200, Jani Nikula wrote: >>> On Tue, 21 Nov 2017, David Weinehall <david.weinehall@linux.intel.com> >wrote: >>> > On Tue, Nov 21, 2017 at 01:51:29PM +0200, Jani Nikula wrote: >>> > > Use i915.dmc_firmware_path to override default firmware for the >>> > > platform and bypassing version checks. >>> > > >>> > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> >>> > > >>> > > --- >>> > > >>> > > Untested. >>> > >>> > Yeah, it kind of shows. It fails to compile :D >>> >>> Oops. I was anxious to get the patch on the list in the heat of IRC >>> discussion, and just added the disclaimer instead of doing this >>> properly... *blush* >>> >>> > >>> > But if you chuck in: >>> > >>> > param(char *, dmc_firmware_path, NULL) \ >> >> I think I already raised my suggestion earlier, but here it goes again. >> Maybe we should consider consolidating these into: >> >> i915.firmware=guc:whatever.bin i915.firmware=dmc:whatever.bin > >I don't think that's possible. I didn't double check but I think you only get one >value from the module param code. Unless you roll your own modparam hooks, >which is best avoided. > >> Then something like "i915.firmware=guc:disable" could be supported, >> too. > >i915.dmc_firmware_path="" could mean disable. > >BR, >Jani. > > >> >> Regards, Joonas >> >>> > >>> > in i915_params.h >>> > >>> > Things work correctly and you can use: >>> > >>> > Tested-by: David Weinehall <david.weinehall@linux.intel.com> >>> > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> >>> >>> Thanks. But let's wait for more input on whether this is really what >>> people want. Perhaps this is useful *regardless* of the outcome of >>> the other discussions. >>> >>> BR, >>> Jani. >>> >>> >>> > >>> > > --- >>> > > drivers/gpu/drm/i915/i915_params.c | 3 +++ >>> > > drivers/gpu/drm/i915/intel_csr.c | 9 +++++++-- >>> > > 2 files changed, 10 insertions(+), 2 deletions(-) >>> > > >>> > > diff --git a/drivers/gpu/drm/i915/i915_params.c >>> > > b/drivers/gpu/drm/i915/i915_params.c >>> > > index 3328147b4863..c11ad6d67fa9 100644 >>> > > --- a/drivers/gpu/drm/i915/i915_params.c >>> > > +++ b/drivers/gpu/drm/i915/i915_params.c >>> > > @@ -171,6 +171,9 @@ i915_param_named_unsafe(guc_firmware_path, >>> > > charp, 0400, i915_param_named_unsafe(huc_firmware_path, charp, >0400, >>> > > "HuC firmware path to use instead of the default one"); >>> > > >>> > > +i915_param_named_unsafe(dmc_firmware_path, charp, 0400, >>> > > + "DMC firmware path to use instead of the default one"); >>> > > + >>> > > i915_param_named_unsafe(enable_dp_mst, bool, 0600, >>> > > "Enable multi-stream transport (MST) for new DisplayPort sinks. >>> > > (default: true)"); >>> > > >>> > > diff --git a/drivers/gpu/drm/i915/intel_csr.c >>> > > b/drivers/gpu/drm/i915/intel_csr.c >>> > > index 77d8b3d483ca..82db376ec7e1 100644 >>> > > --- a/drivers/gpu/drm/i915/intel_csr.c >>> > > +++ b/drivers/gpu/drm/i915/intel_csr.c >>> > > @@ -296,7 +296,10 @@ static uint32_t *parse_csr_fw(struct >>> > > drm_i915_private *dev_priv, >>> > > >>> > > csr->version = css_header->version; >>> > > >>> > > - if (IS_CANNONLAKE(dev_priv)) { >>> > > + if (csr->fw_path == i915_modparams.dmc_firmware_path) { >>> > > + /* Bypass version check for firmware override. */ >>> > > + required_version = csr->version; >>> > > + } else if (IS_CANNONLAKE(dev_priv)) { >>> > > required_version = CNL_CSR_VERSION_REQUIRED; >>> > > } else if (IS_GEMINILAKE(dev_priv)) { >>> > > required_version = GLK_CSR_VERSION_REQUIRED; @@ -451,7 >+454,9 >>> > > @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) >>> > > if (!HAS_CSR(dev_priv)) >>> > > return; >>> > > >>> > > - if (IS_CANNONLAKE(dev_priv)) >>> > > + if (i915_modparams.dmc_firmware_path) >>> > > + csr->fw_path = i915_modparams.dmc_firmware_path; >>> > > + else if (IS_CANNONLAKE(dev_priv)) >>> > > csr->fw_path = I915_CSR_CNL; >>> > > else if (IS_GEMINILAKE(dev_priv)) >>> > > csr->fw_path = I915_CSR_GLK; >>> > > -- >>> > > 2.11.0 >>> > > >>> > > _______________________________________________ >>> > > Intel-gfx mailing list >>> > > Intel-gfx@lists.freedesktop.org >>> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >>> >>> > >-- >Jani Nikula, Intel Open Source Technology 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: add support for specifying DMC firmware override by module param 2017-11-21 21:54 ` Jani Nikula 2017-11-22 8:05 ` Joonas Lahtinen @ 2017-11-22 16:06 ` David Weinehall 1 sibling, 0 replies; 9+ messages in thread From: David Weinehall @ 2017-11-22 16:06 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx On Tue, Nov 21, 2017 at 11:54:16PM +0200, Jani Nikula wrote: > On Tue, 21 Nov 2017, David Weinehall <david.weinehall@linux.intel.com> wrote: > > On Tue, Nov 21, 2017 at 01:51:29PM +0200, Jani Nikula wrote: > >> Use i915.dmc_firmware_path to override default firmware for the platform > >> and bypassing version checks. > >> > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > >> > >> --- > >> > >> Untested. > > > > Yeah, it kind of shows. It fails to compile :D > > Oops. I was anxious to get the patch on the list in the heat of IRC > discussion, and just added the disclaimer instead of doing this > properly... *blush* > > > > > But if you chuck in: > > > > param(char *, dmc_firmware_path, NULL) \ > > > > in i915_params.h > > > > Things work correctly and you can use: > > > > Tested-by: David Weinehall <david.weinehall@linux.intel.com> > > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> > > Thanks. But let's wait for more input on whether this is really what > people want. Perhaps this is useful *regardless* of the outcome of the > other discussions. Fine by me. FWIW *we* (as in the PnP testing team) would definitely benefit if this patch got merged, since we'd be able to simplify automated testing of different versions of DMC. > BR, > Jani. > > > > > >> --- > >> drivers/gpu/drm/i915/i915_params.c | 3 +++ > >> drivers/gpu/drm/i915/intel_csr.c | 9 +++++++-- > >> 2 files changed, 10 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > >> index 3328147b4863..c11ad6d67fa9 100644 > >> --- a/drivers/gpu/drm/i915/i915_params.c > >> +++ b/drivers/gpu/drm/i915/i915_params.c > >> @@ -171,6 +171,9 @@ i915_param_named_unsafe(guc_firmware_path, charp, 0400, > >> i915_param_named_unsafe(huc_firmware_path, charp, 0400, > >> "HuC firmware path to use instead of the default one"); > >> > >> +i915_param_named_unsafe(dmc_firmware_path, charp, 0400, > >> + "DMC firmware path to use instead of the default one"); > >> + > >> i915_param_named_unsafe(enable_dp_mst, bool, 0600, > >> "Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)"); > >> > >> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > >> index 77d8b3d483ca..82db376ec7e1 100644 > >> --- a/drivers/gpu/drm/i915/intel_csr.c > >> +++ b/drivers/gpu/drm/i915/intel_csr.c > >> @@ -296,7 +296,10 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, > >> > >> csr->version = css_header->version; > >> > >> - if (IS_CANNONLAKE(dev_priv)) { > >> + if (csr->fw_path == i915_modparams.dmc_firmware_path) { > >> + /* Bypass version check for firmware override. */ > >> + required_version = csr->version; > >> + } else if (IS_CANNONLAKE(dev_priv)) { > >> required_version = CNL_CSR_VERSION_REQUIRED; > >> } else if (IS_GEMINILAKE(dev_priv)) { > >> required_version = GLK_CSR_VERSION_REQUIRED; > >> @@ -451,7 +454,9 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) > >> if (!HAS_CSR(dev_priv)) > >> return; > >> > >> - if (IS_CANNONLAKE(dev_priv)) > >> + if (i915_modparams.dmc_firmware_path) > >> + csr->fw_path = i915_modparams.dmc_firmware_path; > >> + else if (IS_CANNONLAKE(dev_priv)) > >> csr->fw_path = I915_CSR_CNL; > >> else if (IS_GEMINILAKE(dev_priv)) > >> csr->fw_path = I915_CSR_GLK; > >> -- > >> 2.11.0 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: add support for specifying DMC firmware override by module param 2017-11-21 11:51 [PATCH] drm/i915: add support for specifying DMC firmware override by module param Jani Nikula 2017-11-21 13:43 ` David Weinehall @ 2017-11-21 13:48 ` Patchwork 1 sibling, 0 replies; 9+ messages in thread From: Patchwork @ 2017-11-21 13:48 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx == Series Details == Series: drm/i915: add support for specifying DMC firmware override by module param URL : https://patchwork.freedesktop.org/series/34157/ State : failure == Summary == CHK include/config/kernel.release CHK include/generated/uapi/linux/version.h CHK include/generated/utsrelease.h CHK include/generated/bounds.h CHK include/generated/timeconst.h CHK include/generated/asm-offsets.h CALL scripts/checksyscalls.sh DESCEND objtool CHK scripts/mod/devicetable-offsets.h CHK include/generated/compile.h CHK kernel/config_data.h CC [M] drivers/gpu/drm/i915/i915_params.o In file included from ./include/linux/module.h:18:0, from ./include/drm/drmP.h:59, from drivers/gpu/drm/i915/i915_drv.h:47, from drivers/gpu/drm/i915/i915_params.c:26: drivers/gpu/drm/i915/i915_params.c: In function ‘__check_dmc_firmware_path’: drivers/gpu/drm/i915/i915_params.c:32:48: error: ‘struct i915_params’ has no member named ‘dmc_firmware_path’; did you mean ‘guc_firmware_path’? module_param_named_unsafe(name, i915_modparams.name, T, perm); \ ^ ./include/linux/moduleparam.h:347:68: note: in definition of macro ‘__param_check’ static inline type __always_unused *__check_##name(void) { return(p); } ^ ./include/linux/moduleparam.h:157:2: note: in expansion of macro ‘param_check_charp’ param_check_##type(name, &(value)); \ ^~~~~~~~~~~~ drivers/gpu/drm/i915/i915_params.c:32:2: note: in expansion of macro ‘module_param_named_unsafe’ module_param_named_unsafe(name, i915_modparams.name, T, perm); \ ^~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_params.c:174:1: note: in expansion of macro ‘i915_param_named_unsafe’ i915_param_named_unsafe(dmc_firmware_path, charp, 0400, ^~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_params.c: At top level: drivers/gpu/drm/i915/i915_params.c:32:48: error: ‘struct i915_params’ has no member named ‘dmc_firmware_path’; did you mean ‘guc_firmware_path’? module_param_named_unsafe(name, i915_modparams.name, T, perm); \ ^ ./include/linux/moduleparam.h:228:54: note: in definition of macro ‘__module_param_call’ VERIFY_OCTAL_PERMISSIONS(perm), level, flags, { arg } } ^~~ ./include/linux/moduleparam.h:158:2: note: in expansion of macro ‘module_param_cb_unsafe’ module_param_cb_unsafe(name, ¶m_ops_##type, &value, perm); \ ^~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_params.c:32:2: note: in expansion of macro ‘module_param_named_unsafe’ module_param_named_unsafe(name, i915_modparams.name, T, perm); \ ^~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_params.c:174:1: note: in expansion of macro ‘i915_param_named_unsafe’ i915_param_named_unsafe(dmc_firmware_path, charp, 0400, ^~~~~~~~~~~~~~~~~~~~~~~ scripts/Makefile.build:314: recipe for target 'drivers/gpu/drm/i915/i915_params.o' failed make[4]: *** [drivers/gpu/drm/i915/i915_params.o] Error 1 scripts/Makefile.build:573: recipe for target 'drivers/gpu/drm/i915' failed make[3]: *** [drivers/gpu/drm/i915] Error 2 scripts/Makefile.build:573: recipe for target 'drivers/gpu/drm' failed make[2]: *** [drivers/gpu/drm] Error 2 scripts/Makefile.build:573: recipe for target 'drivers/gpu' failed make[1]: *** [drivers/gpu] Error 2 Makefile:1024: recipe for target 'drivers' failed make: *** [drivers] Error 2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-01-10 0:29 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-11-21 11:51 [PATCH] drm/i915: add support for specifying DMC firmware override by module param Jani Nikula 2017-11-21 13:43 ` David Weinehall 2017-11-21 21:54 ` Jani Nikula 2017-11-22 8:05 ` Joonas Lahtinen 2017-11-22 16:20 ` Jani Nikula 2017-12-21 18:32 ` Srivatsa, Anusha 2018-01-10 0:29 ` Srivatsa, Anusha 2017-11-22 16:06 ` David Weinehall 2017-11-21 13:48 ` ✗ Fi.CI.BAT: failure for " Patchwork
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.