Re: [PATCH 2/6] drm/i915: Add macro for SSEU stride calculation

Summers, Stuart
2019-05-01T19:37:56Z

[-- Attachment #1.1: Type: text/plain, Size: 3879 bytes --]

On Wed, 2019-05-01 at 11:11 -0700, Daniele Ceraolo Spurio wrote:
>
> On 5/1/19 8:34 AM, Stuart Summers wrote:
> > Subslice stride and EU stride are calculated multiple times in
> > i915_query. Move this calculation to a macro to reduce code
> > duplication.
> >
> > v2: update headers in intel_sseu.h
> >
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> > ---
> > drivers/gpu/drm/i915/gt/intel_sseu.h | 2 ++
> > drivers/gpu/drm/i915/i915_query.c | 17 ++++++++---------
> > 2 files changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h
> > b/drivers/gpu/drm/i915/gt/intel_sseu.h
> > index 73bc824094e8..c0b16b248d4c 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_sseu.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_sseu.h
> > @@ -8,11 +8,13 @@
> > #define __INTEL_SSEU_H__
> >
> > #include <linux/types.h>
> > +#include <linux/kernel.h>
> >
> > struct drm_i915_private;
> >
> > #define GEN_MAX_SLICES (6) /* CNL upper bound */
> > #define GEN_MAX_SUBSLICES (8) /* ICL upper bound */
> > +#define GEN_SSEU_STRIDE(bits) DIV_ROUND_UP(bits, BITS_PER_BYTE)
>
> What we pass to this macro isn't really a bits count but the maximum
> amount of s/ss/eus. s/bits/max_entry/, or something like that? with
> that:

Makes sense, I'll make the change in the next series post. Thanks for
the review!

-Stuart

>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>
> Daniele
>
> >
> > struct sseu_dev_info {
> > u8 slice_mask;
> > diff --git a/drivers/gpu/drm/i915/i915_query.c
> > b/drivers/gpu/drm/i915/i915_query.c
> > index 782183b78f49..7c1708c22811 100644
> > --- a/drivers/gpu/drm/i915/i915_query.c
> > +++ b/drivers/gpu/drm/i915/i915_query.c
> > @@ -37,6 +37,8 @@ static int query_topology_info(struct
> > drm_i915_private *dev_priv,
> > const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)-
> > >sseu;
> > struct drm_i915_query_topology_info topo;
> > u32 slice_length, subslice_length, eu_length, total_length;
> > + u8 subslice_stride = GEN_SSEU_STRIDE(sseu->max_subslices);
> > + u8 eu_stride = GEN_SSEU_STRIDE(sseu->max_eus_per_subslice);
> > int ret;
> >
> > if (query_item->flags != 0)
> > @@ -48,12 +50,10 @@ static int query_topology_info(struct
> > drm_i915_private *dev_priv,
> > BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask));
> >
> > slice_length = sizeof(sseu->slice_mask);
> > - subslice_length = sseu->max_slices *
> > - DIV_ROUND_UP(sseu->max_subslices, BITS_PER_BYTE);
> > - eu_length = sseu->max_slices * sseu->max_subslices *
> > - DIV_ROUND_UP(sseu->max_eus_per_subslice,
> > BITS_PER_BYTE);
> > -
> > - total_length = sizeof(topo) + slice_length + subslice_length +
> > eu_length;
> > + subslice_length = sseu->max_slices * subslice_stride;
> > + eu_length = sseu->max_slices * sseu->max_subslices * eu_stride;
> > + total_length = sizeof(topo) + slice_length + subslice_length +
> > + eu_length;
> >
> > ret = copy_query_item(&topo, sizeof(topo), total_length,
> > query_item);
> > @@ -69,10 +69,9 @@ static int query_topology_info(struct
> > drm_i915_private *dev_priv,
> > topo.max_eus_per_subslice = sseu->max_eus_per_subslice;
> >
> > topo.subslice_offset = slice_length;
> > - topo.subslice_stride = DIV_ROUND_UP(sseu->max_subslices,
> > BITS_PER_BYTE);
> > + topo.subslice_stride = subslice_stride;
> > topo.eu_offset = slice_length + subslice_length;
> > - topo.eu_stride =
> > - DIV_ROUND_UP(sseu->max_eus_per_subslice,
> > BITS_PER_BYTE);
> > + topo.eu_stride = eu_stride;
> >
> > if (__copy_to_user(u64_to_user_ptr(query_item->data_ptr),
> > &topo, sizeof(topo)))
> >

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3270 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]