[Intel-gfx] [PATCH] drm/i915: Don't query PCODE RC6VIDS on platforms not supporting it

[Imre Deak]

On Thu, Feb 08, 2018 at 06:33:03PM +0000, Chris Wilson wrote:
> Quoting Imre Deak (2018-02-08 17:41:02)
> > On BXT/GLK GEN6_PCODE_READ_RC6VIDS fails with
> > MAILBOX_P24C_CC_ILLEGAL_CMD, so don't try to do the query on these
> > platforms. Do it only on SNB, IVB and HSW, where we use this command
> > anyway for RC6 enabling.
> > 
> > Based on my tests the command also succeeds on all LLC platforms, but
> > it's not clear if it's really supported on those (it returns 0 aka 245mv
> > for all RC6 states everywhere except on SNB). BSpec lists the command as
> > supported on SKL+ (see P24C_PCODE_MAILBOX_INTERFACE) but that's clearly
> > incorrect, since on SKL/KBL the same command ID is used for
> > SKL_PCODE_LOAD_HDCP_KEYS. Since the command fails on BXT/GLK, the BSpec
> > command list is also incorrect for those platforms (see
> > P_CR_P24C_PCODE_MAILBOX_INTERFACE_0_2_0_GTTMMADR).
> > 
> > I filed a request to update that info in Bspec, but for now let's
> > assume a minimal set of platforms where the command is supported.
> > 
> > Reference: https://bugs.freedesktop.org/show_bug.cgi?id=103337
> > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 24 +++++++++++++++---------
> >  1 file changed, 15 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 2bdce9fea671..8f9045fae3cf 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -1484,9 +1484,12 @@ static int gen6_drpc_info(struct seq_file *m)
> >                 gen9_powergate_status = I915_READ(GEN9_PWRGT_DOMAIN_STATUS);
> >         }
> >  
> > -       mutex_lock(&dev_priv->pcu_lock);
> > -       sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids);
> > -       mutex_unlock(&dev_priv->pcu_lock);
> > +       if (INTEL_GEN(dev_priv) <= 7) {
> > +               mutex_lock(&dev_priv->pcu_lock);
> > +               sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS,
> > +                                      &rc6vids);
> > +               mutex_unlock(&dev_priv->pcu_lock);
> > +       }
> >  
> >         seq_printf(m, "RC1e Enabled: %s\n",
> >                    yesno(rcctl1 & GEN6_RC_CTL_RC1e_ENABLE));
> > @@ -1542,12 +1545,15 @@ static int gen6_drpc_info(struct seq_file *m)
> >         print_rc6_res(m, "RC6+ residency since boot:", GEN6_GT_GFX_RC6p);
> >         print_rc6_res(m, "RC6++ residency since boot:", GEN6_GT_GFX_RC6pp);
> >  
> > -       seq_printf(m, "RC6   voltage: %dmV\n",
> > -                  GEN6_DECODE_RC6_VID(((rc6vids >> 0) & 0xff)));
> > -       seq_printf(m, "RC6+  voltage: %dmV\n",
> > -                  GEN6_DECODE_RC6_VID(((rc6vids >> 8) & 0xff)));
> > -       seq_printf(m, "RC6++ voltage: %dmV\n",
> > -                  GEN6_DECODE_RC6_VID(((rc6vids >> 16) & 0xff)));
> > +       if (INTEL_GEN(dev_priv) <= 7) {
> 
> I expect this will confuse gcc-4.4 and it will emit a
> maybe-used-uninitialised. I would set rc6vids=0 and then test if
> (rc6vids) here.

It's inited already to 0, so shouldn't warn. (0 is a valid value
otherwise).

--Imre

> -Chris