Re: [PATCH 09/14] drm/i915: Introduce i9xx_check_plane_surface()

From: Chris Wilson <chris@chris-wilson.co.uk>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 09/14] drm/i915: Introduce i9xx_check_plane_surface()
Date: Mon, 20 Mar 2017 17:33:08 +0000	[thread overview]
Message-ID: <20170320173308.GK8004@nuc-i3427.alporthouse.com> (raw)
In-Reply-To: <20170320170755.GY31595@intel.com>

On Mon, Mar 20, 2017 at 07:07:55PM +0200, Ville Syrjälä wrote:
> On Fri, Mar 17, 2017 at 10:04:32PM +0000, Chris Wilson wrote:
> > On Fri, Mar 17, 2017 at 11:18:03PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Extract the primary plane surfae offset/x/y calculations for
> > > pre-SKL platforms into a common function, and call it during the
> > > atomic check phase to reduce the amount of stuff we have to do
> > > during the commit phase. SKL is already doing this.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 82 ++++++++++++++++++++++--------------
> > >  1 file changed, 50 insertions(+), 32 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 2e0106a11f8f..024614cb47b6 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3026,6 +3026,43 @@ static u32 i9xx_plane_ctl(const struct intel_crtc_state *crtc_state,
> > >  	return dspcntr;
> > >  }
> > >  
> > > +static int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
> > > +{
> > > +	struct drm_i915_private *dev_priv =
> > > +		to_i915(plane_state->base.plane->dev);
> > > +	int src_x = plane_state->base.src.x1 >> 16;
> > > +	int src_y = plane_state->base.src.y1 >> 16;
> > > +	u32 offset;
> > > +
> > > +	intel_add_fb_offsets(&src_x, &src_y, plane_state, 0);
> > > +
> > > +	if (INTEL_GEN(dev_priv) >= 4)
> > > +		offset = intel_compute_tile_offset(&src_x, &src_y,
> > > +						   plane_state, 0);
> > > +	else
> > > +		offset = 0;
> > > +
> > > +	/* HSW+ does this automagically in hardware */
> > > +	if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv)) {
> > 
> > if (INTEL_GEN() <= 7 && !IS_HASWELL()) {
> > 
> > would match the comment better.
> 
> That would leave out CHV.
> 
> I think 'HAS_GMCH || IS_GEN5 || IS_GEN6 || IS_IVB' might be
> a semi-decent way to put this. But it's still not quite as
> succinct as '!HSW && !BDW'.
> 
> What about if I just change the comment to "HSW/BDW do this ..."?

Prevents me showing my ignorance in that chv isn't include in that set.
Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx