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

2020-11-18 Thread James Ettle

On 28/09/2020 21:12, Kazlauskas, Nicholas wrote:


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.


What about using the existing "video=" kernel command-line option? I 
looks like this acts at the DRM level and is a driver-neutral mechanism 
for specifying defaults on a per-connector basis.


It also has a straightforward means for parsing additional arbitrary 
options that could be used -- from what I can make out this would 
require adding pixel encoding to struct drm_cmdline_mode.


Regards,

-James
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

2020-09-28 Thread James Ettle
On Mon, 2020-09-28 at 16:32 -0400, Alex Deucher wrote:
> Yes, but module parameters global for all devices claimed by that
> driver.  E.g., if you have a system with 2 or 3 AMD GPUs in it, that
> will be applied to all of them.  I think if we want to be able to set
> KMS properties on the kernel command line, we should add the
> functionality to the drm core so it works for all drivers and
> properties.  We already have this for basic modesetting.
> 
> Alex

Thanks for clarifying that -- I see what was meant now.

Personally I'm not wedded to the module parameter, but I think if this
is to be of use to those who are experiencing problems it does need
*some* way of specifying the encoding pretty early on (for things like
boot splashes and GDM -- which I believe is on Wayland).

-James


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

2020-09-28 Thread Alex Deucher
On Mon, Sep 28, 2020 at 4:24 PM James Ettle  wrote:
>
> On Mon, 2020-09-28 at 16:12 -0400, Kazlauskas, Nicholas wrote:
> >
> > 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.
>
> I just want to point out that the module parameter does support
> connector-specific values, e.g.
>
>   amdgpu.pixel_encoding=HDMI-A-1:rgb
>
> should only affect the connector named HDMI-A-1 (unless I've coded it
> wrong -- I don't have enough video ports to test it thoroughly). If
> there's no such-named connector the parameter should be ignored. In
> contrast with no named connector given, e.g.
>
>   amdgpu.pixel_encoding=rgb
>
> should match any connector. Multiple connectors can be given comma-
> separated. I admit the solution here is crude -- it just stores the
> string and rescans it when required.

Yes, but module parameters global for all devices claimed by that
driver.  E.g., if you have a system with 2 or 3 AMD GPUs in it, that
will be applied to all of them.  I think if we want to be able to set
KMS properties on the kernel command line, we should add the
functionality to the drm core so it works for all drivers and
properties.  We already have this for basic modesetting.

Alex
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

2020-09-28 Thread James Ettle
On Mon, 2020-09-28 at 16:12 -0400, Kazlauskas, Nicholas wrote:
> 
> 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.

I just want to point out that the module parameter does support
connector-specific values, e.g.

  amdgpu.pixel_encoding=HDMI-A-1:rgb

should only affect the connector named HDMI-A-1 (unless I've coded it
wrong -- I don't have enough video ports to test it thoroughly). If
there's no such-named connector the parameter should be ignored. In
contrast with no named connector given, e.g.

  amdgpu.pixel_encoding=rgb

should match any connector. Multiple connectors can be given comma-
separated. I admit the solution here is crude -- it just stores the
string and rescans it when required.

-James


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

2020-09-28 Thread 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 
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx 





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

2020-09-28 Thread Christian König

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



-James


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

2020-09-28 Thread 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 
> > 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


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

2020-09-28 Thread Harry Wentland



On 2020-09-25 5:18 p.m., Alex Deucher wrote:
> On Tue, Sep 22, 2020 at 4:51 PM James Ettle  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
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

2020-09-25 Thread Alex Deucher
On Tue, Sep 22, 2020 at 4:51 PM James Ettle  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.

Alex
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

2020-09-22 Thread James Ettle
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? 

-James

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

2020-09-22 Thread Alex Deucher
On Tue, Sep 22, 2020 at 3:34 PM James Ettle  wrote:
>
> Exposes the pixel encoding as the DRM property "pixel encoding"
> and the module parameter pixel_encoding.
>
> The DRM property may take values "auto" (current behaviour
> when a display is plugged in), "rgb" for RGB, or "ycbcr444" for
> YCbCr 4:4:4.
>
> The module parameter may only be set on boot and is of the format
> [connector-name:]encoding[,...] where encoding is one of the
> same values as are valid for the DRM property.
>
> https://gitlab.freedesktop.org/drm/amd/-/issues/476
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |  33 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.h   |   4 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |   8 +
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 247 --
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   1 +
>  drivers/gpu/drm/amd/display/dc/core/dc.c  |  12 +-
>  drivers/gpu/drm/amd/display/dc/dc_stream.h|   2 +
>  7 files changed, 289 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index d76172965199..37748f35c52b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -614,6 +614,31 @@ static const struct drm_prop_enum_list 
> amdgpu_dither_enum_list[] =
> { AMDGPU_FMT_DITHER_ENABLE, "on" },
>  };
>
> +static const struct drm_prop_enum_list amdgpu_user_pixenc_list[] =
> +{  { AMDGPU_USER_PIXENC_AUTO, "auto" },
> +   { AMDGPU_USER_PIXENC_RGB, "rgb" },
> +   { AMDGPU_USER_PIXENC_YCBCR444, "ycbcr444" },
> +};
> +
> +bool amdgpu_user_pixenc_from_name(
> +   enum amdgpu_user_pixenc *user_pixenc,
> +   const char *pixenc_name)
> +{
> +   /* user_pixenc only modified if name found */
> +   bool found = false;
> +   if (pixenc_name && (*pixenc_name != '\0')) {
> +   const int sz = ARRAY_SIZE(amdgpu_user_pixenc_list);
> +   int i;
> +   for (i = 0; !found && i < sz; ++i) {
> +   if (strcmp(pixenc_name, 
> amdgpu_user_pixenc_list[i].name) == 0) {
> +   *user_pixenc = 
> amdgpu_user_pixenc_list[i].type;
> +   found = true;
> +   }
> +   }
> +   }
> +   return found;
> +}
> +
>  int amdgpu_display_modeset_create_props(struct amdgpu_device *adev)
>  {
> int sz;
> @@ -666,6 +691,14 @@ int amdgpu_display_modeset_create_props(struct 
> amdgpu_device *adev)
> "abm level", 0, 4);
> if (!adev->mode_info.abm_level_property)
> return -ENOMEM;
> +
> +   sz = ARRAY_SIZE(amdgpu_user_pixenc_list);
> +   adev->mode_info.pixel_encoding_property =
> +   drm_property_create_enum(adev->ddev, 0,
> +   "pixel encoding",
> +   amdgpu_user_pixenc_list, sz);
> +   if (!adev->mode_info.pixel_encoding_property)
> +   return -ENOMEM;
> }
>
> return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
> index 3620b24785e1..72f82e3f4e32 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
> @@ -45,4 +45,8 @@ amdgpu_display_user_framebuffer_create(struct drm_device 
> *dev,
>struct drm_file *file_priv,
>const struct drm_mode_fb_cmd2 
> *mode_cmd);
>
> +bool amdgpu_user_pixenc_from_name(
> +   enum amdgpu_user_pixenc *user_pixenc,
> +   const char *pixenc_name);
> +
>  #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index 37ba07e2feb5..3c21ecf3d259 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -123,6 +123,12 @@ enum amdgpu_flip_status {
> AMDGPU_FLIP_SUBMITTED
>  };
>
> +enum amdgpu_user_pixenc {
> +   AMDGPU_USER_PIXENC_AUTO,
> +   AMDGPU_USER_PIXENC_RGB,
> +   AMDGPU_USER_PIXENC_YCBCR444
> +};
> +
>  #define AMDGPU_MAX_I2C_BUS 16
>
>  /* amdgpu gpio-based i2c
> @@ -333,6 +339,8 @@ struct amdgpu_mode_info {
> struct drm_property *dither_property;
> /* Adaptive Backlight Modulation (power feature) */
> struct drm_property *abm_level_property;
> +   /* User HDMI pixel encoding override */
> +   struct drm_property *pixel_encoding_property;
> /* hardcoded DFP edid from BIOS */
> struct edid *bios_hardcoded_edid;
> int bios_hardcoded_edid_size;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index b51c527a3f0d..8901271d1902 100644
> --- 

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

2020-09-22 Thread James Ettle
Exposes the pixel encoding as the DRM property "pixel encoding"
and the module parameter pixel_encoding.

The DRM property may take values "auto" (current behaviour
when a display is plugged in), "rgb" for RGB, or "ycbcr444" for
YCbCr 4:4:4.

The module parameter may only be set on boot and is of the format
[connector-name:]encoding[,...] where encoding is one of the
same values as are valid for the DRM property.

https://gitlab.freedesktop.org/drm/amd/-/issues/476
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |  33 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.h   |   4 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |   8 +
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 247 --
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   1 +
 drivers/gpu/drm/amd/display/dc/core/dc.c  |  12 +-
 drivers/gpu/drm/amd/display/dc/dc_stream.h|   2 +
 7 files changed, 289 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index d76172965199..37748f35c52b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -614,6 +614,31 @@ static const struct drm_prop_enum_list 
amdgpu_dither_enum_list[] =
{ AMDGPU_FMT_DITHER_ENABLE, "on" },
 };
 
+static const struct drm_prop_enum_list amdgpu_user_pixenc_list[] =
+{  { AMDGPU_USER_PIXENC_AUTO, "auto" },
+   { AMDGPU_USER_PIXENC_RGB, "rgb" },
+   { AMDGPU_USER_PIXENC_YCBCR444, "ycbcr444" },
+};
+
+bool amdgpu_user_pixenc_from_name(
+   enum amdgpu_user_pixenc *user_pixenc,
+   const char *pixenc_name)
+{
+   /* user_pixenc only modified if name found */
+   bool found = false;
+   if (pixenc_name && (*pixenc_name != '\0')) {
+   const int sz = ARRAY_SIZE(amdgpu_user_pixenc_list);
+   int i;
+   for (i = 0; !found && i < sz; ++i) {
+   if (strcmp(pixenc_name, 
amdgpu_user_pixenc_list[i].name) == 0) {
+   *user_pixenc = amdgpu_user_pixenc_list[i].type;
+   found = true;
+   }
+   }
+   }
+   return found;
+}
+
 int amdgpu_display_modeset_create_props(struct amdgpu_device *adev)
 {
int sz;
@@ -666,6 +691,14 @@ int amdgpu_display_modeset_create_props(struct 
amdgpu_device *adev)
"abm level", 0, 4);
if (!adev->mode_info.abm_level_property)
return -ENOMEM;
+
+   sz = ARRAY_SIZE(amdgpu_user_pixenc_list);
+   adev->mode_info.pixel_encoding_property =
+   drm_property_create_enum(adev->ddev, 0,
+   "pixel encoding",
+   amdgpu_user_pixenc_list, sz);
+   if (!adev->mode_info.pixel_encoding_property)
+   return -ENOMEM;
}
 
return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
index 3620b24785e1..72f82e3f4e32 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
@@ -45,4 +45,8 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,
   struct drm_file *file_priv,
   const struct drm_mode_fb_cmd2 *mode_cmd);
 
+bool amdgpu_user_pixenc_from_name(
+   enum amdgpu_user_pixenc *user_pixenc,
+   const char *pixenc_name);
+
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 37ba07e2feb5..3c21ecf3d259 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -123,6 +123,12 @@ enum amdgpu_flip_status {
AMDGPU_FLIP_SUBMITTED
 };
 
+enum amdgpu_user_pixenc {
+   AMDGPU_USER_PIXENC_AUTO,
+   AMDGPU_USER_PIXENC_RGB,
+   AMDGPU_USER_PIXENC_YCBCR444
+};
+
 #define AMDGPU_MAX_I2C_BUS 16
 
 /* amdgpu gpio-based i2c
@@ -333,6 +339,8 @@ struct amdgpu_mode_info {
struct drm_property *dither_property;
/* Adaptive Backlight Modulation (power feature) */
struct drm_property *abm_level_property;
+   /* User HDMI pixel encoding override */
+   struct drm_property *pixel_encoding_property;
/* hardcoded DFP edid from BIOS */
struct edid *bios_hardcoded_edid;
int bios_hardcoded_edid_size;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index b51c527a3f0d..8901271d1902 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -158,7 +158,6 @@ static int amdgpu_dm_connector_get_modes(struct 
drm_connector *connector);
 static int amdgpu_dm_atomic_commit(struct drm_device *dev,
   struct