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

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Apr 11, 2014 at 07:17:41AM -0700, Matt Roper wrote:
> 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...

Hm ... yeah I guess you're right. For now it probably won't matter too
much.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch