Re: [PATCH] drm/stm: ltdc: fix data enable polarity

From: Philippe CORNU <philippe.cornu@st.com>
To: Mukesh Ojha <mojha@codeaurora.org>,
	Yannick FERTRE <yannick.fertre@st.com>,
	Vincent ABRIOU <vincent.abriou@st.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Alexandre TORGUE <alexandre.torgue@st.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linux-stm32@st-md-mailman.stormreply.com"
	<linux-stm32@st-md-mailman.stormreply.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] drm/stm: ltdc: fix data enable polarity
Date: Mon, 1 Apr 2019 09:18:30 +0000	[thread overview]
Message-ID: <aeb9a5b0-0ad7-7bd1-eaf3-d037c2fb39cf@st.com> (raw)
In-Reply-To: <98a9609b-138b-fc5b-c2b2-46f465c88bab@codeaurora.org>

On 3/29/19 10:32 PM, Mukesh Ojha wrote:
> 
> On 3/29/2019 9:20 PM, Yannick Fertré wrote:
>> Wrong DISPLAY_FLAGS used to set the data enable polarity.
> Used or checked?
> Can you also explain how it is wrong to check against this FLAG in commit?

Dear Yannick,
Many thanks for your patch,

Dear Mukesh,
Many thanks for your comment,

Looking deeper in the stm32 LTDC Ref man chapter, we can read:

Bit 30 VSPOL: vertical synchronization polarity
This bit is set and cleared by software.
0: vertical synchronization is active low.
1: vertical synchronization is active high.

Bit 29 DEPOL: not data enable polarity
This bit is set and cleared by software.
0: not data enable polarity is active low.
1: not data enable polarity is active high.

So I suggest the following commit message:
"According to the STM32 LTDC documentation, GCR_DEPOL bit need to be set 
with DISPLAY_FLAGS_DE_LOW but not with DISPLAY_FLAGS_DE_HIGH."

With that or something similar

Acked-by: Philippe Cornu <philippe.cornu@st.com>

Philippe :-)

>>
>> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
>> ---
>>    drivers/gpu/drm/stm/ltdc.c | 2 +-
>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
>> index b1741a9..6ba326a 100644
>> --- a/drivers/gpu/drm/stm/ltdc.c
>> +++ b/drivers/gpu/drm/stm/ltdc.c
>> @@ -555,7 +555,7 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
>>    	if (vm.flags & DISPLAY_FLAGS_VSYNC_HIGH)
>>    		val |= GCR_VSPOL;
>>    
>> -	if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
>> +	if (vm.flags & DISPLAY_FLAGS_DE_LOW)
>>    		val |= GCR_DEPOL;
>>    
>>    	if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel