| Message ID | 20190802205134.303-4-stuart.summers@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Refactor to expand subslice mask (rev 2) | expand |
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@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. -Chris
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@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. Thanks, Stuart > -Chris
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@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
On Fri, 2019-08-02 at 23:54 +0100, Chris Wilson wrote: > 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@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[]. Bah, obvious mistake on my part here. Thanks for pointing this out. I'll clean this up in the next revision. Thanks, Stuart > -Chris
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); break; } @@ -584,7 +585,7 @@ static void haswell_sseu_info_init(struct drm_i915_private *dev_priv) } intel_sseu_set_info(sseu, hweight8(sseu->slice_mask), - hweight8(sseu->subslice_mask[0]), + hweight8(subslice_mask), sseu->eu_per_subslice); for (s = 0; s < sseu->max_slices; s++) {
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@intel.com> --- drivers/gpu/drm/i915/intel_device_info.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)