Re: [PATCH] drm/i915: Intel-specific primary plane handling (v2)

[Matt Roper <matthew.d.roper@intel.com>]

On Fri, Apr 11, 2014 at 11:34:36AM +0200, Daniel Vetter wrote:
> On Thu, Apr 10, 2014 at 05:24:36PM -0700, Matt Roper wrote:
...
> > +	/* setplane API takes shifted source rectangle values; unshift them */
> > +	src_x >>= 16;
> > +	src_y >>= 16;
> > +	src_w >>= 16;
> > +	src_h >>= 16;
> > +
> > +	/*
> > +	 * Current hardware can't reposition the primary plane or scale it
> > +	 * (although this could change in the future).
> > +	 */
> > +	drm_rect_intersect(&dest, &clip);
> > +	if (dest.x1 != 0 || dest.y1 != 0 ||
> > +	    dest.x2 != crtc->mode.hdisplay || dest.y2 != crtc->mode.vdisplay) {
> > +		DRM_DEBUG_KMS("Primary plane must cover entire CRTC\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (crtc_w != src_w || crtc_h != src_h) {
> > +		DRM_DEBUG_KMS("Can't scale primary plane\n");
> > +		return -EINVAL;
> > +	}
> 
> Subpixel check seems to be missing. And can't we extract all these checks
> both here and from the primary plane helper? I guess there'll be other hw
> which doesn't have scaling primary planes, but which wants to allow
> primary plane enable/disable.

I was a bit unsure about this.  At first I thought I needed to check the
subpixel part, but the DocBook reference indicates

        Devices that don't support subpixel plane coordinates can ignore
        the fractional part.

which sounds to me like we're supposed to just silently ignore the
subpixel bits on i915 and other devices that don't support it.  Which
would probably also mean that I should remove the (subpixel bits == 0)
test from the primary helper...


Matt

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795