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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
> > > ---
> > >  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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux