From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753599AbdKXNyV (ORCPT ); Fri, 24 Nov 2017 08:54:21 -0500 Received: from mx07-00178001.pphosted.com ([62.209.51.94]:64743 "EHLO mx07-00178001.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752619AbdKXNyT (ORCPT ); Fri, 24 Nov 2017 08:54:19 -0500 From: Philippe CORNU To: Peter Rosin , Yannick FERTRE , Benjamin Gaignard , Vincent ABRIOU , David Airlie , "dri-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" CC: Fabien DESSENNE , Mickael REULIER , Gabriel FERNANDEZ , Ludovic BARRE , Alexandre TORGUE , Maxime Coquelin Subject: Re: [PATCH] drm/stm: ltdc: add clut mode support Thread-Topic: [PATCH] drm/stm: ltdc: add clut mode support Thread-Index: AQHTTkwVVliCm0cvvEaL9k/9FO2Iz6MJE7aAgAALX4CABLEbAIAC5rEAgAFziACAEX+bAA== Date: Fri, 24 Nov 2017 13:54:05 +0000 Message-ID: <8f0a5138-e074-e374-102e-76f1bab52b2e@st.com> References: <1509016666-18927-1-git-send-email-philippe.cornu@st.com> <210ab847-7e09-3962-934c-2f0810da409b@axentia.se> <19d562e6-2d1a-0e66-e3f5-390bee1073ad@st.com> <8c428fe5-836c-24b5-3d94-7a294db49bf4@axentia.se> <0b4cac6d-6f3a-7f60-a2fb-873898c9d5e4@st.com> In-Reply-To: <0b4cac6d-6f3a-7f60-a2fb-873898c9d5e4@st.com> Accept-Language: fr-FR, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: user-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 x-ms-exchange-messagesentrepresentingtype: 1 x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.75.127.51] Content-Type: text/plain; charset="utf-8" Content-ID: <415DB18D44AEE14AA57A817C488CA0BC@st.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-11-24_05:,, signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by nfs id vAODsP1M029004 Hi Peter, On 11/13/2017 11:40 AM, Philippe CORNU wrote: > Hi Peter, > > On 11/12/2017 01:31 PM, Peter Rosin wrote: >> On 2017-11-10 17:12, Philippe CORNU wrote: >>> Hi Peter, >>> >>> On 11/07/2017 05:34 PM, Peter Rosin wrote: >>>> On 2017-11-07 16:53, Philippe CORNU wrote: >>>>> + Peter >>>>> >>>>> Hi Peter, >>>>> >>>>> CLUT support on STM32 has been removed thanks to your clean up patch >>>> >>>> Support is a bit strong for what I thought was a dead function, or >>>> are you saying that it used to work before my series? Really sorry >>>> if that is the case! >>> >>> As I wrote in the previous related thread >>> (https://lists.freedesktop.org/archives/dri-devel/2017-June/145070.html), >>> >>> STM32 chipsets supports 8-bit CLUT mode but this driver version does not >>> support it "yet"... >>> >>> So, no worry regarding your clean up, I gave you an "acked-by" for >>> that : ) >> >> Ok, good. Thanks for clearing that up! >> >>>> >>>> Anyway, the function I removed seemed to indicate that the hardware >>>> could handle a separate clut for each layer, but your new version >>>> does not. Why is that? >>> >>> Yes I confirm the clut support is available for each layer... but I >>> thought the gamma_lut was only at the crtc level, not at layer level... >>> Maybe I am wrong. >>> Moreover, small test applications I used play only with clut at crtc >>> level... >>> >>> Anyway, could you please help me to "find" a per-layer clut >>> implementation because when I read "crtc->state->gamma_lut->data" it >>> looks like gamma_lut is per crtc, not per plane...? or maybe I have to >>> add extra properties for that... >> >> I wasn't clear enough. Yes, there is to my knowledge only one clut, >> not one per plane. What I noticed was that the function I removed >> seemed to touch clut registers for multiple layers, but your new >> function appears to only touch registers for one layer. So, I >> wondered if the "one and only" clut needed to be copied to the >> registers for the other layers, or if the old dead code was simply >> confused. Clearer? >> > > The old code was a generic helper function (ie. for all layers) but used > only for the 1st layer. So, we could say that "old dead code was simply > confused" :-) > > When I put back the clut support in this patch, I decided to update only > the 1st layer (because there is no API for handling it on other layers). > I also decided to not re-use the former generic helper function as the > update loop is pretty small. > > This patch offers the clut mode feature for fbdev (only one plane in > fbdev) and for drm (single plane for many use cases, 2nd plane being > used mostly for video...) > > If tomorrow the API offers clut support per plane, the update loop will > be moved to the plane update function, means the generic helper function > will not be require anymore too. From the explanations above, do you think the patch is "acceptable" or should I change it somehow? What is your opinion? Many thanks, Philippe :-) > > Many thanks > Philippe :) > >> Cheers. >> Peter >>