[Intel-gfx] [PATCH 03/10] drm/i915: Use local variable for subslice_mask on HSW

Chris Wilson chris at chris-wilson.co.uk
Fri Aug 2 22:54:53 UTC 2019


Quoting Stuart Summers (2019-08-02 23:47:00)
> On Fri, 2019-08-02 at 22:28 +0100, Chris Wilson wrote:
> > Quoting Stuart Summers (2019-08-02 21:51:27)
> > > Instead of assuming a single slice on HSW when defining
> > > subslices for the platform, use a local variable to set
> > > the maximum subslices per slice.
> > > 
> > > Signed-off-by: Stuart Summers <stuart.summers at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_device_info.c | 11 ++++++-----
> > >  1 file changed, 6 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> > > b/drivers/gpu/drm/i915/intel_device_info.c
> > > index 9a79d9d547c5..2b81cc731fa2 100644
> > > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > > @@ -541,6 +541,7 @@ static void haswell_sseu_info_init(struct
> > > drm_i915_private *dev_priv)
> > >  {
> > >         struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
> > >         u32 fuse1;
> > > +       u8 subslice_mask;
> > >         int s, ss;
> > >  
> > >         /*
> > > @@ -553,16 +554,16 @@ static void haswell_sseu_info_init(struct
> > > drm_i915_private *dev_priv)
> > >                 /* fall through */
> > >         case 1:
> > >                 sseu->slice_mask = BIT(0);
> > > -               sseu->subslice_mask[0] = BIT(0);
> > > +               subslice_mask = BIT(0);
> > >                 break;
> > >         case 2:
> > >                 sseu->slice_mask = BIT(0);
> > > -               sseu->subslice_mask[0] = BIT(0) | BIT(1);
> > > +               subslice_mask = BIT(0) | BIT(1);
> > >                 break;
> > >         case 3:
> > >                 sseu->slice_mask = BIT(0) | BIT(1);
> > > -               sseu->subslice_mask[0] = BIT(0) | BIT(1);
> > > -               sseu->subslice_mask[1] = BIT(0) | BIT(1);
> > > +               subslice_mask = BIT(0) | BIT(1);
> > > +               subslice_mask = BIT(0) | BIT(1);
> > 
> > This is definitely not a single slice.
> 
> Thanks for the note Chris. Very true and my commit message is
> misleading. Do you have any issue with the code changes I'm making
> here? Or simply the commit message? I'll adjust the commit message in
> the next revision.

The duplication looks very wrong, just remove one of them and the reader
isn't left wondering why??? At the moment, it makes me question whether
there is loss of information with an incomplete subslice_mask[].
-Chris


More information about the Intel-gfx mailing list