Re: [PATCH v2 1/5] dt-bindings: display: panel: Move flip properties to panel-common

[Jakob Hauser <jahau@rocketmail.com>]

Hi Krzysztof,

On 23.10.24 09:15, Krzysztof Kozlowski wrote:
> On Tue, Oct 22, 2024 at 11:33:35PM +0200, Jakob Hauser wrote:
>> The flip properties were used by "samsung,s6e8aa0.yaml" only so far. By
>> introducing "samsung,s6e88a0-ams427ap24.yaml" they become more common.
>>
>> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
>> ---
>> Patch is based on current branch drm-misc-next.
>> ---
>>   .../bindings/display/panel/panel-common.yaml           |  8 ++++++++
>>   .../bindings/display/panel/samsung,s6e8aa0.yaml        | 10 ++--------
>>   2 files changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/panel/panel-common.yaml b/Documentation/devicetree/bindings/display/panel/panel-common.yaml
>> index 0a57a31f4f3d..087415753d60 100644
>> --- a/Documentation/devicetree/bindings/display/panel/panel-common.yaml
>> +++ b/Documentation/devicetree/bindings/display/panel/panel-common.yaml
>> @@ -51,6 +51,14 @@ properties:
>>       $ref: /schemas/types.yaml#/definitions/uint32
>>       enum: [0, 90, 180, 270]
>>   
>> +  flip-horizontal:
>> +    description: boolean to flip image horizontally
>> +    type: boolean
>> +
>> +  flip-vertical:
>> +    description: boolean to flip image vertically
>> +    type: boolean
>> +
> 
> I actually wonder how or why would you need to flip the images. I
> understand rotation, but this is mirror flip, right? Is it for some
> transparent displays?

Yes, it's mirror flip. The two cases we have here so far are displays 
for smartphones.

The panel "samsung-s6e88a0-ams427ap24", which this patchset is for, is 
used in device "samsung-serranove" (Samsung Galaxy S4 Mini "Value 
Edition"). By default this panel shows a mirrored picture, it needs flip 
to fix that. I can't think of any good reason and it seems rather 
unique. Maybe there was some mistake during fast-paced development and 
easier to fix that way instead of changing suppliers specification, or 
some purchase related topic, but that's just wild guessing. I 
implemented the flip as an option because I don't know if this driver 
could be extended for similar panels that might not need the flip, e.g. 
older device "samsung-serranolte" (Samsung Galaxy S4 Mini LTE).

For panel "samsung-s6e8aa0" I can't say much because I'm not familiar 
with that. Within the kernel it is used by devices 
"exynos4412-galaxy-s3" (Samsung Galaxy S III) and "exynos4210-trats" 
(don't know what this is). Looking into their devicetree files they 
apply horizontal and vertical flip at once, actually that's a rotation 
by 180 degrees. Looking into the panel driver it seems that this is what 
the hardware interface offers.

> The change is fine, I just really wonder about hardware.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

...

Kind regards,
Jakob