| Message ID | 20180815113016.14964-1-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Fix PM refcounting w/o DMC firmware | expand |
Quoting Imre Deak (2018-08-15 12:30:16) > The case where the firmware isn't specified for a platform (although > runtime PM works only with DMC on this platform) is the same case where > the firmware is specified but can't be loaded for some reason. Hence we > need to get a display init power domain ref in the first case too to > keep the refcount bookkeeping in balance. > > Also convert the related log message to be a debug one, since it's a > valid scenario for a new platform, where we need to have > dev_info->has_csr=1 set, but add support for actually loading the > firmware only later. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=107382 > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/intel_csr.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > index cf9b600cca79..b0384b9d1c58 100644 > --- a/drivers/gpu/drm/i915/intel_csr.c > +++ b/drivers/gpu/drm/i915/intel_csr.c > @@ -468,12 +468,6 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) > csr->fw_path = I915_CSR_SKL; > else if (IS_BROXTON(dev_priv)) > csr->fw_path = I915_CSR_BXT; > - else { > - DRM_ERROR("Unexpected: no known CSR firmware for platform\n"); > - return; > - } > - > - DRM_DEBUG_KMS("Loading %s\n", csr->fw_path); > > /* > * Obtain a runtime pm reference, until CSR is loaded, > @@ -481,6 +475,12 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) > */ > intel_display_power_get(dev_priv, POWER_DOMAIN_INIT); > > + if (csr->fw_path == NULL) { > + DRM_DEBUG_KMS("No known CSR firmware for platform, disabling runtime PM\n"); WARN_ON(!INTEL_INFO(dev_priv)->is_alpha_support) ? -Chris
Quoting Chris Wilson (2018-08-15 12:35:29) > Quoting Imre Deak (2018-08-15 12:30:16) > > The case where the firmware isn't specified for a platform (although > > runtime PM works only with DMC on this platform) is the same case where > > the firmware is specified but can't be loaded for some reason. Hence we > > need to get a display init power domain ref in the first case too to > > keep the refcount bookkeeping in balance. > > > > Also convert the related log message to be a debug one, since it's a > > valid scenario for a new platform, where we need to have > > dev_info->has_csr=1 set, but add support for actually loading the > > firmware only later. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=107382 > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/i915/intel_csr.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > > index cf9b600cca79..b0384b9d1c58 100644 > > --- a/drivers/gpu/drm/i915/intel_csr.c > > +++ b/drivers/gpu/drm/i915/intel_csr.c > > @@ -468,12 +468,6 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) > > csr->fw_path = I915_CSR_SKL; > > else if (IS_BROXTON(dev_priv)) > > csr->fw_path = I915_CSR_BXT; > > - else { > > - DRM_ERROR("Unexpected: no known CSR firmware for platform\n"); > > - return; > > - } > > - > > - DRM_DEBUG_KMS("Loading %s\n", csr->fw_path); > > > > /* > > * Obtain a runtime pm reference, until CSR is loaded, > > @@ -481,6 +475,12 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) > > */ > > intel_display_power_get(dev_priv, POWER_DOMAIN_INIT); > > > > + if (csr->fw_path == NULL) { > > + DRM_DEBUG_KMS("No known CSR firmware for platform, disabling runtime PM\n"); > > WARN_ON(!INTEL_INFO(dev_priv)->is_alpha_support) ? I think it's a sensible reminder in case we forget before attempting to clear the alpha support flag, nevertheless Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Wed, Aug 15, 2018 at 12:38:00PM +0100, Chris Wilson wrote: > Quoting Chris Wilson (2018-08-15 12:35:29) > > Quoting Imre Deak (2018-08-15 12:30:16) > > > The case where the firmware isn't specified for a platform (although > > > runtime PM works only with DMC on this platform) is the same case where > > > the firmware is specified but can't be loaded for some reason. Hence we > > > need to get a display init power domain ref in the first case too to > > > keep the refcount bookkeeping in balance. > > > > > > Also convert the related log message to be a debug one, since it's a > > > valid scenario for a new platform, where we need to have > > > dev_info->has_csr=1 set, but add support for actually loading the > > > firmware only later. > > > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=107382 > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_csr.c | 12 ++++++------ > > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > > > index cf9b600cca79..b0384b9d1c58 100644 > > > --- a/drivers/gpu/drm/i915/intel_csr.c > > > +++ b/drivers/gpu/drm/i915/intel_csr.c > > > @@ -468,12 +468,6 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) > > > csr->fw_path = I915_CSR_SKL; > > > else if (IS_BROXTON(dev_priv)) > > > csr->fw_path = I915_CSR_BXT; > > > - else { > > > - DRM_ERROR("Unexpected: no known CSR firmware for platform\n"); > > > - return; > > > - } > > > - > > > - DRM_DEBUG_KMS("Loading %s\n", csr->fw_path); > > > > > > /* > > > * Obtain a runtime pm reference, until CSR is loaded, > > > @@ -481,6 +475,12 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) > > > */ > > > intel_display_power_get(dev_priv, POWER_DOMAIN_INIT); > > > > > > + if (csr->fw_path == NULL) { > > > + DRM_DEBUG_KMS("No known CSR firmware for platform, disabling runtime PM\n"); > > > > WARN_ON(!INTEL_INFO(dev_priv)->is_alpha_support) ? > > I think it's a sensible reminder in case we forget before attempting to > clear the alpha support flag, Yep, makes sense, will add that. > nevertheless > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > -Chris
On Wed, Aug 15, 2018 at 01:46:01PM +0000, Patchwork wrote: > == Series Details == > > Series: drm/i915: Fix PM refcounting w/o DMC firmware (rev2) > URL : https://patchwork.freedesktop.org/series/48252/ > State : success Pushed to -dinq, thanks for the review. > > == Summary == > > = CI Bug Log - changes from CI_DRM_4671 -> Patchwork_9953 = > > == Summary - SUCCESS == > > No regressions found. > > External URL: https://patchwork.freedesktop.org/api/1.0/series/48252/revisions/2/mbox/ > > == Possible new issues == > > Here are the unknown changes that may have been introduced in Patchwork_9953: > > === IGT changes === > > ==== Possible regressions ==== > > {igt@pm_rpm@module-reload}: > fi-bxt-j4205: SKIP -> DMESG-FAIL > > > == Known issues == > > Here are the changes found in Patchwork_9953 that come from known issues: > > === IGT changes === > > ==== Issues hit ==== > > {igt@amdgpu/amd_prime@i915-to-amd}: > fi-bxt-j4205: SKIP -> INCOMPLETE (fdo#103927) > > > ==== Possible fixes ==== > > igt@kms_frontbuffer_tracking@basic: > fi-hsw-peppy: DMESG-FAIL (fdo#102614) -> PASS > > > {name}: This element is suppressed. This means it is ignored when computing > the status of the difference (SUCCESS, WARNING, or FAILURE). > > fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614 > fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 > > > == Participating hosts (53 -> 47) == > > Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-gdg-551 > > > == Build changes == > > * Linux: CI_DRM_4671 -> Patchwork_9953 > > CI_DRM_4671: 77a98fa3e9b6eb29d513b1666ecddfdcfc424e86 @ git://anongit.freedesktop.org/gfx-ci/linux > IGT_4598: 9c0f04355107a8693650b16756b6343a78501138 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools > Patchwork_9953: a4a5d48f719b9e1c2f3fa9d91ee3204b3768bfe4 @ git://anongit.freedesktop.org/gfx-ci/linux > > > == Linux commits == > > a4a5d48f719b drm/i915: Fix PM refcounting w/o DMC firmware > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9953/issues.html
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index cf9b600cca79..b0384b9d1c58 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -468,12 +468,6 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) csr->fw_path = I915_CSR_SKL; else if (IS_BROXTON(dev_priv)) csr->fw_path = I915_CSR_BXT; - else { - DRM_ERROR("Unexpected: no known CSR firmware for platform\n"); - return; - } - - DRM_DEBUG_KMS("Loading %s\n", csr->fw_path); /* * Obtain a runtime pm reference, until CSR is loaded, @@ -481,6 +475,12 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) */ intel_display_power_get(dev_priv, POWER_DOMAIN_INIT); + if (csr->fw_path == NULL) { + DRM_DEBUG_KMS("No known CSR firmware for platform, disabling runtime PM\n"); + return; + } + + DRM_DEBUG_KMS("Loading %s\n", csr->fw_path); schedule_work(&dev_priv->csr.work); }
The case where the firmware isn't specified for a platform (although runtime PM works only with DMC on this platform) is the same case where the firmware is specified but can't be loaded for some reason. Hence we need to get a display init power domain ref in the first case too to keep the refcount bookkeeping in balance. Also convert the related log message to be a debug one, since it's a valid scenario for a new platform, where we need to have dev_info->has_csr=1 set, but add support for actually loading the firmware only later. References: https://bugs.freedesktop.org/show_bug.cgi?id=107382 Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/intel_csr.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)