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

[Peter Rosin]

On 2017-06-17 07:36, Boris Brezillon wrote:
> Le Sat, 17 Jun 2017 00:46:12 +0200,
> Peter Rosin <peda@xxxxxxxxxx> 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.

Ah, it was the other way around. I had too new code!

Anyway, I just sent a new series which I think addresses all issues except
that I have still not tested with plain drm ioctls.

Cheers,
peda