Re: [RFC PATCH 1/3] atmel-hlcdc: add support for 8-bit color lookup table mode

[Boris Brezillon <boris.brezillon@free-electrons.com>]

Le Sat, 17 Jun 2017 00:46:12 +0200,
Peter Rosin <peda@axentia.se> a écrit :

> >>>> Hm, it's probably too late to do it here. Planes have already been
> >>>> enabled and the engine may have started to fetch data and do the
> >>>> composition. You could do that in ->update_plane() [1], and make it a
> >>>> per-plane thing.
> >>>>
> >>>> I'm not sure, but I think you can get the new crtc_state from
> >>>> plane->crtc->state in this context (state have already been swapped,
> >>>> and new state is being applied, which means relevant locks are held).    
> >>>
> >>> Ok, I can move it there. My plan is to just copy the default .update_plane
> >>> function and insert 
> >>>
> >>> 	if (crtc->state->color_mgmt_changed && crtc->state->gamma_lut) {
> >>> 		...
> >>> 	}
> >>>
> >>> just before the drm_atomic_commit(state) call. Sounds ok?  
> >>
> >> Why would you copy the default ->update_plane() when we already have
> >> our own ->atomic_update_plane() implementation [1]? Just put it there
> >> (before the atmel_hlcdc_layer_update_commit() call) and we should be
> >> good.  
> > 
> > Ahh, but you said ->update_plane() and I took that as .update_plane in
> > layer_plane_funcs, not ->atomic_update() in atmel_hlcdc_layer_plane_helper_funcs.
> > 
> > Makes sense now, and much neater too.  
> 
> No, it doesn't make sense. There's no atmel_hlcdc_layer_update_commit call
> anywhere, and no such function. You seem to have some further changes that
> are not even in -next. Where am I getting those changes and why are they
> not upstream yet?

My bad, this part as been reworked in 4.12 and I was reading 4.11 code.
Indeed, atmel_hlcdc_layer_update_commit() no longer exists, but
atmel_hlcdc_plane_atomic_update() does.

Just add a function called atmel_hlcdc_plane_update_clut() in
atmel_hlcdc_plane.c and call it just after [1].

[1]http://elixir.free-electrons.com/linux/v4.12-rc5/source/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c#L770