Re: [PATCH 03/18] drm/i915: Add .max_stride() plane hook

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Aug 22, 2018 at 10:19:55PM +0000, Souza, Jose wrote:
> On Wed, 2018-08-22 at 22:03 +0000, Souza, Jose wrote:
> > On Thu, 2018-07-19 at 21:21 +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Each plane may have different stride limitations. Let's add a new
> > > plane function to retutn the maximum stride for each plane. There's
> > > going to be some use for this outside the .atomic_check() stuff
> > > hence
> > > the separate hook.
> > 
> > I just not checked the the spec for the VLV and CHV other than that
> > LGTM but just take a look to the nitpick bellow:
> > 
> > Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> > 
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 46
> > > ++++++++++++++++++++++++++++++++++++
> > >  drivers/gpu/drm/i915/intel_drv.h     | 10 ++++++++
> > >  drivers/gpu/drm/i915/intel_sprite.c  | 34
> > > ++++++++++++++++++++++++--
> > >  3 files changed, 88 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 5f8304a11482..a09e11e0596f 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3210,6 +3210,31 @@ int skl_check_plane_surface(const struct
> > > intel_crtc_state *crtc_state,
> > >  	return 0;
> > >  }
> > >  
> > > +unsigned int
> > > +i9xx_plane_max_stride(struct intel_plane *plane,
> > > +		      u32 pixel_format, u64 modifier,
> > > +		      unsigned int rotation)
> > > +{
> > > +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > > +
> 
> Reviwing 'drm/i915: Use pipe A primary plane .max_stride() as the
> global stride limit' I notice that this case was not moved here:
> 
> } else if (gen >= 5 && !HAS_GMCH_DISPLAY(dev_priv)) {
>                return 32*1024;
> }
> 
> Not relevant anymore?

No, that should not have been dropped. Good catch. I'll respin.

> 
> 
> > > +	if (INTEL_GEN(dev_priv) >= 4) {
> > > +		if (modifier == I915_FORMAT_MOD_X_TILED)
> > > +			return 16*1024;
> > > +		else
> > > +			return 32*1024;
> > > +	} else if (INTEL_GEN(dev_priv) >= 3) {
> > > +		if (modifier == I915_FORMAT_MOD_X_TILED)
> > > +			return 8*1024;
> > > +		else
> > > +			return 16*1024;
> > > +	} else {
> > > +		if (plane->i9xx_plane == PLANE_C)
> > > +			return 4*1024;
> > > +		else
> > > +			return 8*1024;
> > > +	}
> > > +}
> > > +

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx