Re: [PATCH 4/9] drm/i915: split intel_update_plane into check() and commit()

[Gustavo Padovan <gustavo@padovan.org>]

2014-08-29 Ville Syrjälä <ville.syrjala@linux.intel.com>:

> On Thu, Aug 28, 2014 at 02:40:08PM -0300, Gustavo Padovan wrote:
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > 
> > Due to the upcoming atomic modesetting feature we need to separate
> > some update functions into a check step that can fail and a commit
> > step that should, ideally, never fail.
> > 
> > This commit splits intel_update_plane() and its commit part can still
> > fail due to the fb pinning procedure.
> > 
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > ---
> >  drivers/gpu/drm/i915/intel_sprite.c | 128 ++++++++++++++++++++++++++----------
> >  1 file changed, 93 insertions(+), 35 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 4cbe286..b1cb606 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -844,22 +844,32 @@ static bool colorkey_enabled(struct intel_plane *intel_plane)
> >  	return key.flags != I915_SET_COLORKEY_NONE;
> >  }
> >  
> > +static bool get_visible(struct drm_rect *src, struct drm_rect *dst,
> 
> get_visisble() is not a good name here IMO, also I think this split is at
> a slightly too low level. What we really want is to start creating some
> kind of plane config struct that can be passed to both check and commit,
> and then check can already store all the clipped coordinates etc. to the
> plane config and commit can just look them up w/o recomputing them.

What do you really mean by too low level here? Would grabbing
struct drm_plane_state from the atomic branch fix this for you?

I'll get a v2 of these patches working with struct drm_plane_state.

	Gustavo