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

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Apr 11, 2014 at 10:41:56AM -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:
> ...
> > 
> > Hm, I've thought we could do a simple
> > 
> > 	if (intel_crtc->primary_enabled)
> > 		call_primary_plane_helper
> > 	else
> > 		enable_the_hw_plane
> > 
> > But we need to do all the arg checking for the !primary_enabled case :(
> > Anyway more code sharing make me happier.
> > 
> > Cheers, Daniel
> 
> I think the problem here is that the helper has a bunch of tests
> targetted at the lowest common denominator hardware.  Some of the things
> it rejects are things that our hardware may begin to allow at some point
> in the future (e.g., primary plane scaling, partial CRTC coverage of
> primary plane, etc.).  We can probably call into the helper today and
> get the behavior we want, but I'd expect that some of those restrictions
> will need to be relaxed in the future and then we'll have to switch the
> code back at that point.  Given that we still need to do all this
> checking in the 'if (!enabled)' case, I don't think it's worth trying to
> call through the helper for the 'if (enabled)' case (especially since
> the actual "work" here after we're done testing is just a couple lines
> of code)?

Well for that future I simply expect that we'll get a completely new
update_plane function. I agree that reusing the helper completely doesn't
work really, but sharing the tests would be nice imo.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch