Subject: Re: [RFC PATCH 1/3] atmel-hlcdc: add support for 8-bit color lookup
 table mode
From: Peter Rosin <peda@axentia.se>
To: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: linux-kernel@vger.kernel.org, David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel.vetter@intel.com>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Sean Paul <seanpaul@chromium.org>, dri-devel@lists.freedesktop.org
References: <1497604347-17960-1-git-send-email-peda@axentia.se>
 <1497604347-17960-2-git-send-email-peda@axentia.se>
 <20170616120148.28c9caca@bbrezillon>
 <2f78bacf-b779-676d-3fbd-c49a7c851788@axentia.se>
 <20170616181519.37757934@bbrezillon>
 <c597ed29-996c-1312-9f4c-907f0b0b8b36@axentia.se>
Organization: Axentia Technologies AB
Message-ID: <88384c85-6115-6833-663c-6e9ca802d867@axentia.se>
Date: Sat, 17 Jun 2017 00:46:12 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.1.1
MIME-Version: 1.0
In-Reply-To: <c597ed29-996c-1312-9f4c-907f0b0b8b36@axentia.se>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Jun 2017 22:46:16.7222 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0202MB2549
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1565
Lines: 40

>>>> 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?

There's a mention of the missing function here [1], but that's some 18
months ago. What's going on?

[1] https://patchwork.kernel.org/patch/7965721/

Cheers,
peda