Re: [PATCH 1/2] drm/i915/bxt: Determine BXT slice/subslice/EU info

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Apr 02, 2015 at 02:50:09PM -0700, Jeff McGee wrote:
> On Wed, Apr 01, 2015 at 08:20:44AM +0200, Daniel Vetter wrote:
> > On Tue, Mar 31, 2015 at 09:59:22AM -0700, jeff.mcgee@intel.com wrote:
> > > From: Jeff McGee <jeff.mcgee@intel.com>
> > > 
> > > BXT uses a subset of the SKL fuse registers, because it has at
> > > most 1 slice and at most 6 EU per subslice.
> > > 
> > > Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_dma.c | 47 +++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 47 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > > index ec661fe..81afd31 100644
> > > --- a/drivers/gpu/drm/i915/i915_dma.c
> > > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > @@ -733,6 +733,53 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
> > >  		info->has_slice_pg = (info->slice_total > 1) ? 1 : 0;
> > >  		info->has_subslice_pg = 0;
> > >  		info->has_eu_pg = (info->eu_per_subslice > 2) ? 1 : 0;
> > > +	} else if (IS_BROXTON(dev)) {
> > 
> > By all reasonable standards this function is getting a bit too long. Can
> > you please create a patch to extract the various platform-specific sseu
> > detection logic.
> > 
> > Also can't we just repurpose the skl version by limiting s_max
> > appropriately and applying
> > 
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index 68e0c85a17cf..b164aeb09158 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -668,9 +668,8 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
> >  		ss_disable = (fuse2 & GEN9_F2_SS_DIS_MASK) >>
> >  			     GEN9_F2_SS_DIS_SHIFT;
> >  
> > -		eu_disable[0] = I915_READ(GEN8_EU_DISABLE0);
> > -		eu_disable[1] = I915_READ(GEN8_EU_DISABLE1);
> > -		eu_disable[2] = I915_READ(GEN8_EU_DISABLE2);
> > +		for (s = 0; s < s_max; s++)
> > +			eu_disable[s] = I915_READ(GEN8_EU_DISABLE(s));
> >  
> >  		info->slice_total = hweight32(s_enable);
> > 
> > with a suitable added #define ofc?
> > -Daniel
> 
> Agree with all of the above. Regarding the 2nd patch in this series which
> performs related logic in i915_debugfs, would you like to see that broken
> up similarly?

Yeah I think the same rework would benefit the debugfs code too, if only
for consistency.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx