[Intel-gfx] [PATCH] drm/i915: Fix PM refcounting w/o DMC firmware

[Imre Deak]

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 at chris-wilson.co.uk>
> > > Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
> > > Signed-off-by: Imre Deak <imre.deak at 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 at chris-wilson.co.uk>

> -Chris