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

[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.]

-James