[PATCH] drm/amdgpu/dc: Pixel encoding DRM property and module parameter

[Kazlauskas, Nicholas]

On 2020-09-28 3:31 p.m., Christian König wrote:
> Am 28.09.20 um 19:35 schrieb James Ettle:
>> On Mon, 2020-09-28 at 10:26 -0400, Harry Wentland wrote:
>>> On 2020-09-25 5:18 p.m., Alex Deucher wrote:
>>>> On Tue, Sep 22, 2020 at 4:51 PM James Ettle <james at ettle.org.uk>
>>>> wrote:
>>>>> On 22/09/2020 21:33, Alex Deucher wrote:
>>>>>>> +/**
>>>>>>> + * DOC: pixel_encoding (string)
>>>>>>> + * Specify the initial pixel encoding used by a connector.
>>>>>>> + */
>>>>>>> +static char amdgpu_pixel_encoding[MAX_INPUT];
>>>>>>> +MODULE_PARM_DESC(pixel_encoding, "Override pixel encoding");
>>>>>>> +module_param_string(pixel_encoding, amdgpu_pixel_encoding,
>>>>>>> sizeof(amdgpu_pixel_encoding), 0444);
>>>>>> You can drop this part.  We don't need a module parameter if we
>>>>>> have a
>>>>>> kms property.
>>>>>>
>>>>>> Alex
>>>>> OK, but is there then an alternative means of setting the pixel
>>>>> encoding to be used immediately on boot or when amdgpu loads?
>>>>> Also are there user tools other than xrandr to change a KMS
>>>>> property, for Wayland and console users?
>>>> You can force some things on the kernel command line, but I don't
>>>> recall whether that includes kms properties or not.  As for ways to
>>>> change properties, the KMS API provides a way.  those are exposed
>>>> via
>>>> randr when using X.  When using wayland compositors, it depends on
>>>> the
>>>> compositor.
>>>>
>>> I'm not aware of a way to specify KMS properties through the kernel
>>> command line. I don't think it's possible.
>>>
>>> For atomic userspace the userspace wants to be the authority on the
>>> KMS
>>> config. I'm not sure there's a way to set these properties with
>>> Wayland
>>> unless a Wayland compositor plumbs them through.
>>>
>>> Can you summarize on a higher level what problem you're trying to
>>> solve?
>>> I wonder if it'd be better solved with properties on a DRM level that
>>> all drivers can follow if desired.
>>>
>>> Harry
>>>
>>>> Alex
>>>>
>> The problem this is trying to solve is that the pixel format defaults
>> to YCbCr444 on HDMI if the monitor claims to support it, in preference
>> to RGB. This behaviour is hard-coded (see the
>> comment fill_stream_properties_from_drm_display_mode) and there is no
>> way for the user to change the pixel format to RGB, other than hacking
>> the EDID to disable the YCbCr flags.
>>
>> Using YCbCr (rather than RGB) has been reported to cause suboptimal
>> results for some users: colour quality issues or perceptible conversion
>> latency at the monitor end -- see:
>>
>> https://gitlab.freedesktop.org/drm/amd/-/issues/476 
>>
>>
>> for the full details.
>>
>> This patch tries to solve this issue by adding a DRM property so Xorg
>> users can change the pixel encoding on-the-fly, and a module parameter
>> to set the default encoding at amdgpu's init for all users.
>>
>> [I suppose an alternative here is to look into the rationale of
>> defaulting to YCbCr444 on HDMI when the monitor also supports RGB. For
>> reference although on my kit I see no detrimental effects from YCbCr,
>> I'm using onboard graphics with a motherboard that has just D-sub and
>> HDMI -- so DisplayPort's not an option.]
> 
> Ah, that problem again. Yes, that's an issue since the early fglrx days 
> on linux.
> 
> Shouldn't the pixel encoding be part of the mode to run ?
> 
> Regards,
> Christian.

DRM modes don't specify the encoding. The property as part of this patch 
lets userspace override it but the userspace GUI support isn't there on 
Wayland IIRC.

I'm fine with adding the properties but I don't think the module 
parameter is the right solution here. I think it's better if we try to 
get this into atomic userspace as well or revive efforts that have been 
already started before.

The problem with the module parameters is that it'd be applying a 
default to every DRM connector. No way to specify different defaults per 
DRM connector, nor do we know the full connector set at driver 
initialization. The list is dynamic and can change when you plug/unplug 
MST displays.

Regards,
Nicholas Kazlauskas

> 
>>
>> -James
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx 
>>
>