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

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Apr 22, 2014 at 08:18:30AM -0700, Matt Roper wrote:
> On Tue, Apr 22, 2014 at 03:47:37PM +0300, Ville Syrjälä wrote:
> > On Fri, Apr 11, 2014 at 02:13:42PM -0700, Matt Roper wrote:
> ...
> > > +	int ret;
> > > +
> > > +	/*
> > > +	 * At the moment we use the same set of setplane restrictions as the
> > > +	 * DRM primary plane helper, so go ahead and just call the helper if
> > > +	 * the primary plane is already enabled.  We only need to take special
> > > +	 * action if the primary plane is disabled (something i915 can do but
> > > +	 * the generic helper can't).
> > > +	 */
> > > +	if (intel_crtc->primary_enabled)
> > > +		return drm_primary_helper_update(plane, crtc, fb,
> > > +						 crtc_x, crtc_y,
> > > +						 crtc_w, crtc_h,
> > > +						 src_x, src_y,
> > > +						 src_w, src_h);
> > 
> > Why would we want to call that if we have a custom implementation
> > anyway?
> 
> This was something Daniel requested on a previous patch iteration; even
> though we're stuck duplicating most of the checks here for the !enabled
> case, he still wanted to see us call into the helper for the enabled
> case (although this will have to change in the future if/when we want to
> start relaxing some of the tests that the helper does, such as plane
> scaling).

To clarify my request: I was unhappy with all the duplicated tests we have
and would like some way to share them with the plane helper code. If
there's no sane way to do that, then I'm ok with duplication.

I'm not sure any more what was the issue with extracting the tests from
the plane helper into a new function and reusing them with i915 though.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch