Re: [PATCH] drm/i915/skl: don't fail colorkey + scaler request

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

On Fri, May 22, 2015 at 08:29:28AM +0200, Daniel Vetter wrote:
> On Thu, May 21, 2015 at 07:36:44PM +0300, Ville Syrjälä wrote:
> > On Mon, May 18, 2015 at 04:18:44PM -0700, Chandra Konduru wrote:
> > > There is a mplayer video failure reported with xv.
> > > This is because there is a request to do both plane scaling
> > > and colorkey. Because skl hw doesn't support plane scaling
> > > and colorkey at the same time, request is failed which is expected
> > > behavior.
> > > 
> > > To make xv operate, this patch allows colorkey continue to work
> > > without using scaler. Then behavior would be similar to platforms
> > > without plane scaler support.
> > > 
> > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90449
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c |   14 +++++++++-----
> > >  drivers/gpu/drm/i915/intel_sprite.c  |   31 +++++++++++++++++++++----------
> > >  2 files changed, 30 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 7ab75c0..fc5cef0 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -4505,9 +4505,10 @@ skl_update_scaler_users(
> > >  	}
> > >  
> > >  	/* check colorkey */
> > > -	if (intel_plane && intel_plane->ckey.flags != I915_SET_COLORKEY_NONE) {
> > > -		DRM_DEBUG_KMS("PLANE:%d scaling with color key not allowed",
> > > -			intel_plane->base.base.id);
> > > +	if (WARN_ON(intel_plane &&
> > > +		intel_plane->ckey.flags != I915_SET_COLORKEY_NONE)) {
> > > +		DRM_DEBUG_KMS("PLANE:%d scaling %ux%u->%ux%u not allowed with colorkey",
> > > +			intel_plane->base.base.id, src_w, src_h, dst_w, dst_h);
> > >  		return -EINVAL;
> > >  	}
> > >  
> > > @@ -13049,8 +13050,11 @@ intel_check_primary_plane(struct drm_plane *plane,
> > >  		intel_atomic_get_crtc_state(state->base.state, intel_crtc) : NULL;
> > >  
> > >  	if (INTEL_INFO(dev)->gen >= 9) {
> > > -		min_scale = 1;
> > > -		max_scale = skl_max_scale(intel_crtc, crtc_state);
> > > +		/* use scaler when colorkey is not required */
> > > +		if (to_intel_plane(plane)->ckey.flags == I915_SET_COLORKEY_NONE) {
> > > +			min_scale = 1;
> > > +			max_scale = skl_max_scale(intel_crtc, crtc_state);
> > > +		}
> > >  		can_position = true;
> > >  	}
> > >  
> > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > index 16d0e48..9004e47 100644
> > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > @@ -775,7 +775,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
> > >  	struct drm_rect *dst = &state->dst;
> > >  	const struct drm_rect *clip = &state->clip;
> > >  	int hscale, vscale;
> > > -	int max_scale, min_scale;
> > > +	int max_scale, min_scale, can_scale;
> > 
> > can_scale ought to be bool. Otherwise this looks good.
> > So with that bikeshed done this gets:
> 
> Done. Aside, intel_check_sprite_plane is a giant function, some extraction
> of sub-parts and helpers (like computing can_scale and stuff) would be
> good.

And some of it should be split up into platform specific checks
since not all platforms have the same restrictions. I think what's
there is generally valid for ILK-IVB but may not be really valid for
others.

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