linux-kernel - Re: [PATCH 02/11] drm/bridge: tc358768: Fix bit updates

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <52151daa-90af-a6c0-9b03-f69081321253@ideasonboard.com>
Date:   Mon, 14 Aug 2023 09:34:04 +0300
From:   Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To:     Maxim Schwalm <maxim.schwalm@...il.com>,
        Péter Ujfalusi <peter.ujfalusi@...il.com>,
        Andrzej Hajda <andrzej.hajda@...el.com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Robert Foss <rfoss@...nel.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Francesco Dolcini <francesco@...cini.it>
Cc:     linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        Aradhya Bhatia <a-bhatia1@...com>
Subject: Re: [PATCH 02/11] drm/bridge: tc358768: Fix bit updates

On 13/08/2023 03:23, Maxim Schwalm wrote:
> Hi,
> 
> On 11.08.23 19:02, Tomi Valkeinen wrote:
>> On 11/08/2023 19:23, Péter Ujfalusi wrote:
>>>
>>>
>>> On 04/08/2023 13:44, Tomi Valkeinen wrote:
>>>> The driver has a few places where it does:
>>>>
>>>> if (thing_is_enabled_in_config)
>>>> 	update_thing_bit_in_hw()
>>>>
>>>> This means that if the thing is _not_ enabled, the bit never gets
>>>> cleared. This affects the h/vsyncs and continuous DSI clock bits.
>>>
>>> I guess the idea was to keep the reset value unless it needs to be flipped.
>>>
>>>>
>>>> Fix the driver to always update the bit.
>>>>
>>>> Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver")
>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
>>>> ---
>>>>    drivers/gpu/drm/bridge/tc358768.c | 13 +++++++------
>>>>    1 file changed, 7 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
>>>> index bc97a837955b..b668f77673c3 100644
>>>> --- a/drivers/gpu/drm/bridge/tc358768.c
>>>> +++ b/drivers/gpu/drm/bridge/tc358768.c
>>>> @@ -794,8 +794,8 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
>>>>    		val |= BIT(i + 1);
>>>>    	tc358768_write(priv, TC358768_HSTXVREGEN, val);
>>>>    
>>>> -	if (!(mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS))
>>>> -		tc358768_write(priv, TC358768_TXOPTIONCNTRL, 0x1);
>>>> +	tc358768_write(priv, TC358768_TXOPTIONCNTRL,
>>>> +		       (mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) ? 0 : BIT(0));
>>>>    
>>>>    	/* TXTAGOCNT[26:16] RXTASURECNT[10:0] */
>>>>    	val = tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk * 4);
>>>> @@ -861,11 +861,12 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
>>>>    	tc358768_write(priv, TC358768_DSI_HACT, hact);
>>>>    
>>>>    	/* VSYNC polarity */
>>>> -	if (!(mode->flags & DRM_MODE_FLAG_NVSYNC))
>>>> -		tc358768_update_bits(priv, TC358768_CONFCTL, BIT(5), BIT(5));
>>>> +	tc358768_update_bits(priv, TC358768_CONFCTL, BIT(5),
>>>> +			     (mode->flags & DRM_MODE_FLAG_PVSYNC) ? BIT(5) : 0);
>>>
>>> Was this the reverse before and should be:
>>> (mode->flags & DRM_MODE_FLAG_PVSYNC) ? 0 : BIT(5)
>>
>> Bit 5 is 1 for active high vsync polarity. The test was previously
>> !nvsync, i.e. the same as pvsync.
> 
> this statement doesn't seem to be true, since this change causes a
> regression on the Asus TF700T. Apparently, !nvsync is true and pvsync is
> false in the present case.

panasonic_vvx10f004b00_mode in panel_simple.c doesn't seem to have mode 
flags set. I would say that means the panel doesn't care about the sync 
polarities (which obviously is not the case), but maybe there's an 
assumption that if sync polarities are not set, the default is... 
positive? But I can't find any mention about this.

Does it work for you if you set the polarities in 
panasonic_vvx10f004b00_mode?

  Tomi