Message-ID: <a3e214fc-43e3-4f95-b73f-5040b733c7a5@rocketmail.com>
Date: Fri, 11 Oct 2024 21:58:05 +0200
From: Jakob Hauser <jahau@...ketmail.com>
To: Jessica Zhang <quic_jesszhan@...cinc.com>
Cc: Neil Armstrong <neil.armstrong@...aro.org>, Rob Herring
 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, dri-devel@...ts.freedesktop.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 ~postmarketos/upstreaming@...ts.sr.ht
Subject: Re: [PATCH 4/4] drm/panel: samsung-s6e88a0-ams427ap24: Add flip
 option

Hi Jessica,

On 11.10.24 19:17, Jessica Zhang wrote:
> 
> On 10/10/2024 11:31 AM, Jakob Hauser wrote:
>> The way of implementing a flip option follows the existing
>> panel-samsung-s6e8aa0.c [1][2][3].
>>
>> The value to flip the screen is taken from a downstream kernel file of
>> a similar but older panel [4]. The mipi clock [5] for the new panel
>> samsung-s6e88a0-ams427ap24 matches 461 MHz and a hardware read-out of the
>> 0xcb values corresponds to revision R01 of that older panel [6]. Although
>> for samsung-s6e88a0-ams427ap24 that's in non-flipped state while in this
>> older driver it seems to be the other way around. Further up there is a
> 
> Hi Jakob,
> 
> I'm a bit confused by the wording here. Do you mean that even though the 
> downstream driver comments state the panel is in a non-flipped state by 
> default, your observations suggest that it's actually defaulting to a 
> flipped state?
> 
> Thanks,
> 
> Jessica Zhang
> 
>> hint [7] basically saying for revision R01 to change the first word of 
>> the
>> 0xcb command from 0x06 to 0x0e, which is actually setting BIT(3) of that
>> word. This causes a horizontal flip.
>>
>> [1] 
>> https://github.com/torvalds/linux/blob/v6.11/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c#L103
>> [2] 
>> https://github.com/torvalds/linux/blob/v6.11/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c#L207-L211
>> [3] 
>> https://github.com/torvalds/linux/blob/v6.11/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c#L954-L974
>> [4] 
>> https://github.com/LineageOS/android_kernel_samsung_msm8930-common/blob/lineage-15.1/drivers/video/msm/mipi_samsung_oled_video_qhd_pt-8930.c
>> [5] 
>> https://github.com/LineageOS/android_kernel_samsung_msm8930-common/blob/lineage-15.1/drivers/video/msm/mipi_samsung_oled_video_qhd_pt-8930.c#L2027-L2028
>> [6] 
>> https://github.com/LineageOS/android_kernel_samsung_msm8930-common/blob/lineage-15.1/drivers/video/msm/mipi_samsung_oled_video_qhd_pt-8930.c#L137-L151
>> [7] 
>> https://github.com/LineageOS/android_kernel_samsung_msm8930-common/blob/lineage-15.1/drivers/video/msm/mipi_samsung_oled_video_qhd_pt-8930.c#L66-L74

In the commit message I'm referencing another downstream driver for a 
different (similar but older) panel. The commit message is very 
summarized. I'll try to describe the situation more detailed here. Maybe 
this is going too far but I don't know how to dissolve it otherwise.

The panel AMS427AP24 of this patchset is mounted in device 
samsung-serranove "Samsung Galaxy S4 Mini Value Edition". On this device 
the picture by default is the wrong way around (flipped/mirrored). So it 
needs horizontal flip to get it right. In the downstream Android kernel 
this is done in the panel controller. Following links are just for 
reference, no need to study them in deep: "hflip" in dtsi file [a], 
reading "hflip" in mdss_dsi_panel.c [b], and then I'm fully not sure how 
it continues... processing in mdss_mdp_overlay.c [d] or in 
mdss_mdp_rotator.c [e] or in mdss_mdp_pipe.c [f].

I noticed that in another downstream panel driver used by the similar 
but older device samsung-serranolte ("Samsung Galaxy S4 Mini LTE", not 
the "ve" Value Edition) the flip is done directly in the panel driver. 
That driver is labelled "AMS427AP01" [f] but it seems to serve a couple 
of different dimensions [g] and also distinguishes between an original 
revision and a revision "r01". This driver contains a section "#if 
defined(CONFIG_FEATURE_FLIPLR)" [h]. FLIPLR means flip left-right. That 
section holds values for the 0xcb command for different sizes (different 
mipi clocks) and the two different revisions.

When reading out the default values of the 0xcb command on the newer 
device samsung-serranove ("ve" Value Edition") with panel AMS427AP24, 
they match with the values of the older driver AMS427AP01 for mipi clock 
461 MHz revision r01 [i].

However, for the newer device samsung-serranove ("ve" Value Edition") 
with panel AMS427AP24 that's the default value. Now it needs a 
horizontal flip to get the picture right. This can be achieved by 
changing the first value of 0xcb command from 0x06 to 0x0e. That's what 
I implemented in the patch as an option.

For the older device samsung-serranolte (LTE, not "ve" Value Edition) 
with panel AMS427AP01 I can't say much. That's possibly where the 
confusion comes from. In that driver for that older panel the hint on 
value 0x0e is just a comment [j] while the value 0x06 [k] is part of the 
"#if defined(CONFIG_FEATURE_FLIPLR)" section. Therefore I assume that on 
the older panel AMS427AP01 it's the other way around: value 0x0e as 
default and 0x06 as flip option.

I hope this more detailed description is comprehensible. Let me know if 
you have questions. Also feel free to suggest improvements on the commit 
message.

[a] 
https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/video/msm/mdss/samsung/S6E88A0_AMS427AP24/dsi_panel_S6E88A0_AMS427AP24_qhd_octa_video.dtsi#L112
[b] 
https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/video/msm/mdss/mdss_dsi_panel.c#L1290-L1291
[c] 
https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/video/msm/mdss/mdss_mdp_overlay.c#L709-L711
[d] 
https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/video/msm/mdss/mdss_mdp_rotator.c#L590-L591
[e] 
https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/video/msm/mdss/mdss_mdp_pipe.c#L1309-L1310
[f] 
https://github.com/LineageOS/android_kernel_samsung_msm8930-common/blob/lineage-15.1/drivers/video/msm/mipi_samsung_oled_video_qhd_pt-8930.c#L1867
[g] 
https://github.com/LineageOS/android_kernel_samsung_msm8930-common/blob/lineage-15.1/drivers/video/msm/mipi_samsung_oled_video_qhd_pt-8930.c#L1980-L1995
[h] 
https://github.com/LineageOS/android_kernel_samsung_msm8930-common/blob/lineage-15.1/drivers/video/msm/mipi_samsung_oled_video_qhd_pt-8930.c#L65-L246
[i] 
https://github.com/LineageOS/android_kernel_samsung_msm8930-common/blob/lineage-15.1/drivers/video/msm/mipi_samsung_oled_video_qhd_pt-8930.c#L137-L151
[j] 
https://github.com/LineageOS/android_kernel_samsung_msm8930-common/blob/lineage-15.1/drivers/video/msm/mipi_samsung_oled_video_qhd_pt-8930.c#L68
[k] 
https://github.com/LineageOS/android_kernel_samsung_msm8930-common/blob/lineage-15.1/drivers/video/msm/mipi_samsung_oled_video_qhd_pt-8930.c#L139-L140

...

Kind regards,
Jakob

Powered by blists - more mailing lists