linux-kernel - Re: [PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066

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: <BF06ES.TD22854ZPLB92@packett.cool>
Date: Mon, 27 May 2024 19:13:59 -0300
From: Val Packett <val@...kett.cool>
To: Heiko Stübner <heiko@...ech.de>
Cc: stable@...r.kernel.org, Sandy Huang <hjc@...k-chips.com>, Andy Yan
	<andy.yan@...k-chips.com>, Maarten Lankhorst
	<maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
	Daniel Vetter <daniel@...ll.ch>, dri-devel@...ts.freedesktop.org,
	linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank
 on RK3066

On Mon, May 27 2024 at 22:43:18 +02:00:00, Heiko Stübner 
<heiko@...ech.de> wrote:
> Hi Val,
> 
> Am Montag, 27. Mai 2024, 09:16:33 CEST schrieb Val Packett:
>>  On the RK3066, there is a bit that must be cleared, otherwise
>>  the picture does not show up
>> on the display (at least for RGB).
>> 
>>  Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
>>  Cc: stable@...r.kernel.org
>>  Signed-off-by: Val Packett <val@...kett.cool>
>>  ---
>>  v2: doing this on vblank makes more sense; added fixes tag
> 
> can you give a rationale for this please?
> 
> I.e. does this dma-stop bit need to be set on each vblank that happens
> to push this frame to the display somehow?

The only things I'm 100% sure about:

- that bit is called dma_stop in the Android kernel's header;
- without ever setting that bit to 1, it was getting set to 1 by the 
chip itself, as logging the register on flush was showing a 1 in that 
position (it was the only set bit - I guess others aren't readable 
after cfg_done?);
- without clearing it "between" frames, the whole screen is always 
filled with noise, the picture is not visible.

The rest is at least a bit (ha) speculative:

As I understand from what the name implies, the hardware sets it to 
indicate that it has scanned out the frame and is waiting for 
acknowledgment (clearing) to be able to scan out the next frame. I 
guess it's a redundant synchronization mechanism that was removed in 
later iterations of the VOP hardware block.

I've been trying to see if moving where I clear the bit affects the 
sort-of-tearing-but-vertical glitches that sometimes happen, especially 
early on after the system has just booted, but that seems to be 
completely unrelated pixel clock craziness (the Android kernel runs the 
screen at 66 fps, interestingly).

I'm fairly confident that both places are "correct". The reason I'm 
more on the side of vblank now is that it made logical sense to me when 
I thought about it more: acknowledging that the frame has been scanned 
out is a reaction to the frame having been scanned out. It's a 
consequence of *that* that the acknowledgment is required for the next 
frame to be drawn.

Unless we can get the opinion of someone closely familiar with this 
decade-old hardware, we only have this reasoning to go off of :)

~val
>