Re: [PATCH v7 21/36] drm/connector: hdmi: Add Broadcast RGB property
On Fri, Mar 01, 2024 at 01:12:02PM +0100, Maxime Ripard wrote: > On Fri, Mar 01, 2024 at 12:29:41PM +0100, Sebastian Wick wrote: > > On Fri, Mar 01, 2024 at 11:30:56AM +0100, Maxime Ripard wrote: > > > On Thu, Feb 29, 2024 at 08:47:26PM +0100, Sebastian Wick wrote: > > > > > @@ -1708,6 +1731,39 @@ > > > > > EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property); > > > > > /** > > > > > * DOC: HDMI connector properties > > > > > * > > > > > + * Broadcast RGB (HDMI specific) > > > > > + * Indicates the Quantization Range (Full vs Limited) used. The > > > > > color > > > > > + * processing pipeline will be adjusted to match the value of > > > > > the > > > > > + * property, and the Infoframes will be generated and sent > > > > > accordingly. > > > > > + * > > > > > + * This property is only relevant if the HDMI output format is > > > > > RGB. If > > > > > + * it's one of the YCbCr variant, it will be ignored and the > > > > > output will > > > > > + * use a limited quantization range. > > > > > > > > Uh, maybe just say that the quantization range is selected automatically > > > > in case a YCbCr output format is in use. I'm not sure every YCbCr > > > > variant requires limited and even if it does, new formats could change > > > > this. > > > > > > I documented what i915 is doing: > > > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/i915/display/intel_hdmi.c#L2143 > > > > Sure, this is one valid strategy for the automatic behavior of YCbCr. > > Drivers could also always send an InfoFrame to ensure full range where > > possible. The point here is that this property shall not affect YCbCr > > output formats! > > > > Maybe it's even better to say "driver specific" instead of "automatic". > > Honestly, I'm not sure what you want from me here. Ville and you > insisted on the previous version to document what i915 is doing and to > follow whatever the behaviour was, and that we shouldn't spend time > improving the property. Fine, I did that. > > But now, you want me to ... improve the property? The property has a clear scope: quantization range for RGB output formats. What Intel does with things that are not in scope of the property is irrelevant. This isn't improving the property either but documenting the scope of the property. Sorry if this seems arbitrary but these details are so important to get right because the interactions between all the different things is already majorly broken. > Maxime >
Re: [PATCH v7 21/36] drm/connector: hdmi: Add Broadcast RGB property
On Fri, 1 Mar 2024 13:12:02 +0100 Maxime Ripard wrote: > On Fri, Mar 01, 2024 at 12:29:41PM +0100, Sebastian Wick wrote: > > On Fri, Mar 01, 2024 at 11:30:56AM +0100, Maxime Ripard wrote: > > > On Thu, Feb 29, 2024 at 08:47:26PM +0100, Sebastian Wick wrote: > > > > > @@ -1708,6 +1731,39 @@ > > > > > EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property); > > > > > /** > > > > > * DOC: HDMI connector properties > > > > > * > > > > > + * Broadcast RGB (HDMI specific) > > > > > + * Indicates the Quantization Range (Full vs Limited) used. The > > > > > color > > > > > + * processing pipeline will be adjusted to match the value of > > > > > the > > > > > + * property, and the Infoframes will be generated and sent > > > > > accordingly. > > > > > + * > > > > > + * This property is only relevant if the HDMI output format is > > > > > RGB. If > > > > > + * it's one of the YCbCr variant, it will be ignored and the > > > > > output will > > > > > + * use a limited quantization range. > > > > > > > > Uh, maybe just say that the quantization range is selected automatically > > > > in case a YCbCr output format is in use. I'm not sure every YCbCr > > > > variant requires limited and even if it does, new formats could change > > > > this. > > > > > > I documented what i915 is doing: > > > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/i915/display/intel_hdmi.c#L2143 > > > > > > > Sure, this is one valid strategy for the automatic behavior of YCbCr. > > Drivers could also always send an InfoFrame to ensure full range where > > possible. The point here is that this property shall not affect YCbCr > > output formats! > > > > Maybe it's even better to say "driver specific" instead of "automatic". > > Honestly, I'm not sure what you want from me here. Ville and you > insisted on the previous version to document what i915 is doing and to > follow whatever the behaviour was, and that we shouldn't spend time > improving the property. Fine, I did that. > > But now, you want me to ... improve the property? Just drop the "and the output will use a limited quantization range" part. Thanks, pq pgp3eUs1UPHj9.pgp Description: OpenPGP digital signature
Re: [PATCH v7 21/36] drm/connector: hdmi: Add Broadcast RGB property
On Fri, Mar 01, 2024 at 12:29:41PM +0100, Sebastian Wick wrote: > On Fri, Mar 01, 2024 at 11:30:56AM +0100, Maxime Ripard wrote: > > On Thu, Feb 29, 2024 at 08:47:26PM +0100, Sebastian Wick wrote: > > > > @@ -1708,6 +1731,39 @@ > > > > EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property); > > > > /** > > > > * DOC: HDMI connector properties > > > > * > > > > + * Broadcast RGB (HDMI specific) > > > > + * Indicates the Quantization Range (Full vs Limited) used. The > > > > color > > > > + * processing pipeline will be adjusted to match the value of the > > > > + * property, and the Infoframes will be generated and sent > > > > accordingly. > > > > + * > > > > + * This property is only relevant if the HDMI output format is > > > > RGB. If > > > > + * it's one of the YCbCr variant, it will be ignored and the > > > > output will > > > > + * use a limited quantization range. > > > > > > Uh, maybe just say that the quantization range is selected automatically > > > in case a YCbCr output format is in use. I'm not sure every YCbCr > > > variant requires limited and even if it does, new formats could change > > > this. > > > > I documented what i915 is doing: > > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/i915/display/intel_hdmi.c#L2143 > > Sure, this is one valid strategy for the automatic behavior of YCbCr. > Drivers could also always send an InfoFrame to ensure full range where > possible. The point here is that this property shall not affect YCbCr > output formats! > > Maybe it's even better to say "driver specific" instead of "automatic". Honestly, I'm not sure what you want from me here. Ville and you insisted on the previous version to document what i915 is doing and to follow whatever the behaviour was, and that we shouldn't spend time improving the property. Fine, I did that. But now, you want me to ... improve the property? Maxime
Re: [PATCH v7 21/36] drm/connector: hdmi: Add Broadcast RGB property
On Fri, Mar 01, 2024 at 11:30:56AM +0100, Maxime Ripard wrote: > On Thu, Feb 29, 2024 at 08:47:26PM +0100, Sebastian Wick wrote: > > > @@ -1708,6 +1731,39 @@ > > > EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property); > > > /** > > > * DOC: HDMI connector properties > > > * > > > + * Broadcast RGB (HDMI specific) > > > + * Indicates the Quantization Range (Full vs Limited) used. The > > > color > > > + * processing pipeline will be adjusted to match the value of the > > > + * property, and the Infoframes will be generated and sent > > > accordingly. > > > + * > > > + * This property is only relevant if the HDMI output format is RGB. > > > If > > > + * it's one of the YCbCr variant, it will be ignored and the output > > > will > > > + * use a limited quantization range. > > > > Uh, maybe just say that the quantization range is selected automatically > > in case a YCbCr output format is in use. I'm not sure every YCbCr > > variant requires limited and even if it does, new formats could change > > this. > > I documented what i915 is doing: > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/i915/display/intel_hdmi.c#L2143 Sure, this is one valid strategy for the automatic behavior of YCbCr. Drivers could also always send an InfoFrame to ensure full range where possible. The point here is that this property shall not affect YCbCr output formats! Maybe it's even better to say "driver specific" instead of "automatic". > Maxime >
Re: [PATCH v7 21/36] drm/connector: hdmi: Add Broadcast RGB property
On Fri, Mar 01, 2024 at 09:29:17AM +0100, Hans Verkuil wrote:
> On 29/02/2024 20:47, Sebastian Wick wrote:
> > On Thu, Feb 22, 2024 at 07:14:07PM +0100, Maxime Ripard wrote:
> >> The i915 driver has a property to force the RGB range of an HDMI output.
> >> The vc4 driver then implemented the same property with the same
> >> semantics. KWin has support for it, and a PR for mutter is also there to
> >> support it.
> >>
> >> Both drivers implementing the same property with the same semantics,
> >> plus the userspace having support for it, is proof enough that it's
> >> pretty much a de-facto standard now and we can provide helpers for it.
> >>
> >> Let's plumb it into the newly created HDMI connector.
> >>
> >> Reviewed-by: Dave Stevenson
> >> Signed-off-by: Maxime Ripard
> >> ---
> >> Documentation/gpu/kms-properties.csv | 1 -
> >> drivers/gpu/drm/drm_atomic.c | 2 +
> >> drivers/gpu/drm/drm_atomic_state_helper.c | 4 +-
> >> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
> >> drivers/gpu/drm/drm_connector.c | 89
> >> +++
> >> include/drm/drm_connector.h | 36 +
> >> 6 files changed, 134 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/gpu/kms-properties.csv
> >> b/Documentation/gpu/kms-properties.csv
> >> index 0f9590834829..caef14c532d4 100644
> >> --- a/Documentation/gpu/kms-properties.csv
> >> +++ b/Documentation/gpu/kms-properties.csv
> >> @@ -17,7 +17,6 @@ Owner Module/Drivers,Group,Property Name,Type,Property
> >> Values,Object attached,De
> >> ,Virtual GPU,“suggested X”,RANGE,"Min=0,
> >> Max=0x",Connector,property to suggest an X offset for a connector
> >> ,,“suggested Y”,RANGE,"Min=0, Max=0x",Connector,property to
> >> suggest an Y offset for a connector
> >> ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9""
> >> }",Connector,TDB
> >> -i915,Generic,"""Broadcast RGB""",ENUM,"{ ""Automatic"", ""Full"",
> >> ""Limited 16:235"" }",Connector,"When this property is set to Limited
> >> 16:235 and CTM is set, the hardware will be programmed with the result of
> >> the multiplication of CTM by the limited range matrix to ensure the pixels
> >> normally in the range 0..1.0 are remapped to the range 16/255..235/255."
> >> ,,“audio”,ENUM,"{ ""force-dvi"", ""off"", ""auto"", ""on""
> >> }",Connector,TBD
> >> ,SDVO-TV,“mode”,ENUM,"{ ""NTSC_M"", ""NTSC_J"", ""NTSC_443"", ""PAL_B"" }
> >> etc.",Connector,TBD
> >> ,,"""left_margin""",RANGE,"Min=0, Max= SDVO dependent",Connector,TBD
> >> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> >> index 26f9e525c0a0..3e57d98d8418 100644
> >> --- a/drivers/gpu/drm/drm_atomic.c
> >> +++ b/drivers/gpu/drm/drm_atomic.c
> >> @@ -1145,6 +1145,8 @@ static void drm_atomic_connector_print_state(struct
> >> drm_printer *p,
> >>
> >>if (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> >>connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
> >> + drm_printf(p, "\tbroadcast_rgb=%s\n",
> >> +
> >> drm_hdmi_connector_get_broadcast_rgb_name(state->hdmi.broadcast_rgb));
> >>drm_printf(p, "\toutput_bpc=%u\n", state->hdmi.output_bpc);
> >>drm_printf(p, "\toutput_format=%s\n",
> >>
> >> drm_hdmi_connector_get_output_format_name(state->hdmi.output_format));
> >> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
> >> b/drivers/gpu/drm/drm_atomic_state_helper.c
> >> index 9f517599f117..0e8fb653965a 100644
> >> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> >> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> >> @@ -589,6 +589,7 @@ void __drm_atomic_helper_connector_hdmi_reset(struct
> >> drm_connector *connector,
> >>
> >>new_state->max_bpc = max_bpc;
> >>new_state->max_requested_bpc = max_bpc;
> >> + new_state->hdmi.broadcast_rgb = DRM_HDMI_BROADCAST_RGB_AUTO;
> >> }
> >> EXPORT_SYMBOL(__drm_atomic_helper_connector_hdmi_reset);
> >>
> >> @@ -913,7 +914,8 @@ int drm_atomic_helper_connector_hdmi_check(struct
> >> drm_connector *connector,
> >>if (ret)
> >>return ret;
> >>
> >> - if (old_state->hdmi.output_bpc != new_state->hdmi.output_bpc ||
> >> + if (old_state->hdmi.broadcast_rgb != new_state->hdmi.broadcast_rgb ||
> >> + old_state->hdmi.output_bpc != new_state->hdmi.output_bpc ||
> >>old_state->hdmi.output_format != new_state->hdmi.output_format) {
> >>struct drm_crtc *crtc = new_state->crtc;
> >>struct drm_crtc_state *crtc_state;
> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> >> b/drivers/gpu/drm/drm_atomic_uapi.c
> >> index 29d4940188d4..2b415b4ed506 100644
> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> >> @@ -776,6 +776,8 @@ static int drm_atomic_connector_set_property(struct
> >> drm_connector *connector,
> >>state->max_requested_bpc = val;
> >>} else
Re: [PATCH v7 21/36] drm/connector: hdmi: Add Broadcast RGB property
On Thu, Feb 29, 2024 at 08:47:26PM +0100, Sebastian Wick wrote: > > @@ -1708,6 +1731,39 @@ > > EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property); > > /** > > * DOC: HDMI connector properties > > * > > + * Broadcast RGB (HDMI specific) > > + * Indicates the Quantization Range (Full vs Limited) used. The color > > + * processing pipeline will be adjusted to match the value of the > > + * property, and the Infoframes will be generated and sent > > accordingly. > > + * > > + * This property is only relevant if the HDMI output format is RGB. If > > + * it's one of the YCbCr variant, it will be ignored and the output > > will > > + * use a limited quantization range. > > Uh, maybe just say that the quantization range is selected automatically > in case a YCbCr output format is in use. I'm not sure every YCbCr > variant requires limited and even if it does, new formats could change > this. I documented what i915 is doing: https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/i915/display/intel_hdmi.c#L2143 Maxime
Re: [PATCH v7 21/36] drm/connector: hdmi: Add Broadcast RGB property
On 29/02/2024 20:47, Sebastian Wick wrote:
> On Thu, Feb 22, 2024 at 07:14:07PM +0100, Maxime Ripard wrote:
>> The i915 driver has a property to force the RGB range of an HDMI output.
>> The vc4 driver then implemented the same property with the same
>> semantics. KWin has support for it, and a PR for mutter is also there to
>> support it.
>>
>> Both drivers implementing the same property with the same semantics,
>> plus the userspace having support for it, is proof enough that it's
>> pretty much a de-facto standard now and we can provide helpers for it.
>>
>> Let's plumb it into the newly created HDMI connector.
>>
>> Reviewed-by: Dave Stevenson
>> Signed-off-by: Maxime Ripard
>> ---
>> Documentation/gpu/kms-properties.csv | 1 -
>> drivers/gpu/drm/drm_atomic.c | 2 +
>> drivers/gpu/drm/drm_atomic_state_helper.c | 4 +-
>> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
>> drivers/gpu/drm/drm_connector.c | 89
>> +++
>> include/drm/drm_connector.h | 36 +
>> 6 files changed, 134 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/gpu/kms-properties.csv
>> b/Documentation/gpu/kms-properties.csv
>> index 0f9590834829..caef14c532d4 100644
>> --- a/Documentation/gpu/kms-properties.csv
>> +++ b/Documentation/gpu/kms-properties.csv
>> @@ -17,7 +17,6 @@ Owner Module/Drivers,Group,Property Name,Type,Property
>> Values,Object attached,De
>> ,Virtual GPU,“suggested X”,RANGE,"Min=0, Max=0x",Connector,property
>> to suggest an X offset for a connector
>> ,,“suggested Y”,RANGE,"Min=0, Max=0x",Connector,property to suggest
>> an Y offset for a connector
>> ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9""
>> }",Connector,TDB
>> -i915,Generic,"""Broadcast RGB""",ENUM,"{ ""Automatic"", ""Full"", ""Limited
>> 16:235"" }",Connector,"When this property is set to Limited 16:235 and CTM
>> is set, the hardware will be programmed with the result of the
>> multiplication of CTM by the limited range matrix to ensure the pixels
>> normally in the range 0..1.0 are remapped to the range 16/255..235/255."
>> ,,“audio”,ENUM,"{ ""force-dvi"", ""off"", ""auto"", ""on"" }",Connector,TBD
>> ,SDVO-TV,“mode”,ENUM,"{ ""NTSC_M"", ""NTSC_J"", ""NTSC_443"", ""PAL_B"" }
>> etc.",Connector,TBD
>> ,,"""left_margin""",RANGE,"Min=0, Max= SDVO dependent",Connector,TBD
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 26f9e525c0a0..3e57d98d8418 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -1145,6 +1145,8 @@ static void drm_atomic_connector_print_state(struct
>> drm_printer *p,
>>
>> if (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
>> connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
>> +drm_printf(p, "\tbroadcast_rgb=%s\n",
>> +
>> drm_hdmi_connector_get_broadcast_rgb_name(state->hdmi.broadcast_rgb));
>> drm_printf(p, "\toutput_bpc=%u\n", state->hdmi.output_bpc);
>> drm_printf(p, "\toutput_format=%s\n",
>>
>> drm_hdmi_connector_get_output_format_name(state->hdmi.output_format));
>> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
>> b/drivers/gpu/drm/drm_atomic_state_helper.c
>> index 9f517599f117..0e8fb653965a 100644
>> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
>> @@ -589,6 +589,7 @@ void __drm_atomic_helper_connector_hdmi_reset(struct
>> drm_connector *connector,
>>
>> new_state->max_bpc = max_bpc;
>> new_state->max_requested_bpc = max_bpc;
>> +new_state->hdmi.broadcast_rgb = DRM_HDMI_BROADCAST_RGB_AUTO;
>> }
>> EXPORT_SYMBOL(__drm_atomic_helper_connector_hdmi_reset);
>>
>> @@ -913,7 +914,8 @@ int drm_atomic_helper_connector_hdmi_check(struct
>> drm_connector *connector,
>> if (ret)
>> return ret;
>>
>> -if (old_state->hdmi.output_bpc != new_state->hdmi.output_bpc ||
>> +if (old_state->hdmi.broadcast_rgb != new_state->hdmi.broadcast_rgb ||
>> +old_state->hdmi.output_bpc != new_state->hdmi.output_bpc ||
>> old_state->hdmi.output_format != new_state->hdmi.output_format) {
>> struct drm_crtc *crtc = new_state->crtc;
>> struct drm_crtc_state *crtc_state;
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
>> b/drivers/gpu/drm/drm_atomic_uapi.c
>> index 29d4940188d4..2b415b4ed506 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -776,6 +776,8 @@ static int drm_atomic_connector_set_property(struct
>> drm_connector *connector,
>> state->max_requested_bpc = val;
>> } else if (property == connector->privacy_screen_sw_state_property) {
>> state->privacy_screen_sw_state = val;
>> +} else if (property == connector->broadcast_rgb_property) {
>> +
Re: [PATCH v7 21/36] drm/connector: hdmi: Add Broadcast RGB property
On Thu, Feb 22, 2024 at 07:14:07PM +0100, Maxime Ripard wrote:
> The i915 driver has a property to force the RGB range of an HDMI output.
> The vc4 driver then implemented the same property with the same
> semantics. KWin has support for it, and a PR for mutter is also there to
> support it.
>
> Both drivers implementing the same property with the same semantics,
> plus the userspace having support for it, is proof enough that it's
> pretty much a de-facto standard now and we can provide helpers for it.
>
> Let's plumb it into the newly created HDMI connector.
>
> Reviewed-by: Dave Stevenson
> Signed-off-by: Maxime Ripard
> ---
> Documentation/gpu/kms-properties.csv | 1 -
> drivers/gpu/drm/drm_atomic.c | 2 +
> drivers/gpu/drm/drm_atomic_state_helper.c | 4 +-
> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
> drivers/gpu/drm/drm_connector.c | 89
> +++
> include/drm/drm_connector.h | 36 +
> 6 files changed, 134 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/gpu/kms-properties.csv
> b/Documentation/gpu/kms-properties.csv
> index 0f9590834829..caef14c532d4 100644
> --- a/Documentation/gpu/kms-properties.csv
> +++ b/Documentation/gpu/kms-properties.csv
> @@ -17,7 +17,6 @@ Owner Module/Drivers,Group,Property Name,Type,Property
> Values,Object attached,De
> ,Virtual GPU,“suggested X”,RANGE,"Min=0, Max=0x",Connector,property
> to suggest an X offset for a connector
> ,,“suggested Y”,RANGE,"Min=0, Max=0x",Connector,property to suggest
> an Y offset for a connector
> ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9""
> }",Connector,TDB
> -i915,Generic,"""Broadcast RGB""",ENUM,"{ ""Automatic"", ""Full"", ""Limited
> 16:235"" }",Connector,"When this property is set to Limited 16:235 and CTM is
> set, the hardware will be programmed with the result of the multiplication of
> CTM by the limited range matrix to ensure the pixels normally in the range
> 0..1.0 are remapped to the range 16/255..235/255."
> ,,“audio”,ENUM,"{ ""force-dvi"", ""off"", ""auto"", ""on"" }",Connector,TBD
> ,SDVO-TV,“mode”,ENUM,"{ ""NTSC_M"", ""NTSC_J"", ""NTSC_443"", ""PAL_B"" }
> etc.",Connector,TBD
> ,,"""left_margin""",RANGE,"Min=0, Max= SDVO dependent",Connector,TBD
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 26f9e525c0a0..3e57d98d8418 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1145,6 +1145,8 @@ static void drm_atomic_connector_print_state(struct
> drm_printer *p,
>
> if (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
> + drm_printf(p, "\tbroadcast_rgb=%s\n",
> +
> drm_hdmi_connector_get_broadcast_rgb_name(state->hdmi.broadcast_rgb));
> drm_printf(p, "\toutput_bpc=%u\n", state->hdmi.output_bpc);
> drm_printf(p, "\toutput_format=%s\n",
>
> drm_hdmi_connector_get_output_format_name(state->hdmi.output_format));
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
> b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 9f517599f117..0e8fb653965a 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -589,6 +589,7 @@ void __drm_atomic_helper_connector_hdmi_reset(struct
> drm_connector *connector,
>
> new_state->max_bpc = max_bpc;
> new_state->max_requested_bpc = max_bpc;
> + new_state->hdmi.broadcast_rgb = DRM_HDMI_BROADCAST_RGB_AUTO;
> }
> EXPORT_SYMBOL(__drm_atomic_helper_connector_hdmi_reset);
>
> @@ -913,7 +914,8 @@ int drm_atomic_helper_connector_hdmi_check(struct
> drm_connector *connector,
> if (ret)
> return ret;
>
> - if (old_state->hdmi.output_bpc != new_state->hdmi.output_bpc ||
> + if (old_state->hdmi.broadcast_rgb != new_state->hdmi.broadcast_rgb ||
> + old_state->hdmi.output_bpc != new_state->hdmi.output_bpc ||
> old_state->hdmi.output_format != new_state->hdmi.output_format) {
> struct drm_crtc *crtc = new_state->crtc;
> struct drm_crtc_state *crtc_state;
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 29d4940188d4..2b415b4ed506 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -776,6 +776,8 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> state->max_requested_bpc = val;
> } else if (property == connector->privacy_screen_sw_state_property) {
> state->privacy_screen_sw_state = val;
> + } else if (property == connector->broadcast_rgb_property) {
> + state->hdmi.broadcast_rgb = val;
> } else if (connector->funcs->atomic_set_property) {
> return
Re: [PATCH v7 21/36] drm/connector: hdmi: Add Broadcast RGB property
On Thu, 22 Feb 2024 19:14:07 +0100
Maxime Ripard wrote:
> The i915 driver has a property to force the RGB range of an HDMI output.
> The vc4 driver then implemented the same property with the same
> semantics. KWin has support for it, and a PR for mutter is also there to
> support it.
>
> Both drivers implementing the same property with the same semantics,
> plus the userspace having support for it, is proof enough that it's
> pretty much a de-facto standard now and we can provide helpers for it.
>
> Let's plumb it into the newly created HDMI connector.
>
> Reviewed-by: Dave Stevenson
> Signed-off-by: Maxime Ripard
> ---
> Documentation/gpu/kms-properties.csv | 1 -
> drivers/gpu/drm/drm_atomic.c | 2 +
> drivers/gpu/drm/drm_atomic_state_helper.c | 4 +-
> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
> drivers/gpu/drm/drm_connector.c | 89
> +++
> include/drm/drm_connector.h | 36 +
> 6 files changed, 134 insertions(+), 2 deletions(-)
...
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 591d2d500f61..6ffe59d01698 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1212,6 +1212,29 @@ static const u32 dp_colorspaces =
> BIT(DRM_MODE_COLORIMETRY_BT2020_CYCC) |
> BIT(DRM_MODE_COLORIMETRY_BT2020_YCC);
>
> +static const struct drm_prop_enum_list broadcast_rgb_names[] = {
> + { DRM_HDMI_BROADCAST_RGB_AUTO, "Automatic" },
> + { DRM_HDMI_BROADCAST_RGB_FULL, "Full" },
> + { DRM_HDMI_BROADCAST_RGB_LIMITED, "Limited 16:235" },
> +};
> +
> +/*
> + * drm_hdmi_connector_get_broadcast_rgb_name - Return a string for HDMI
> connector RGB broadcast selection
> + * @broadcast_rgb: Broadcast RGB selection to compute name of
> + *
> + * Returns: the name of the Broadcast RGB selection, or NULL if the type
> + * is not valid.
> + */
> +const char *
> +drm_hdmi_connector_get_broadcast_rgb_name(enum drm_hdmi_broadcast_rgb
> broadcast_rgb)
> +{
> + if (broadcast_rgb > DRM_HDMI_BROADCAST_RGB_LIMITED)
> + return NULL;
> +
> + return broadcast_rgb_names[broadcast_rgb].name;
> +}
> +EXPORT_SYMBOL(drm_hdmi_connector_get_broadcast_rgb_name);
> +
> static const char * const output_format_str[] = {
> [HDMI_COLORSPACE_RGB] = "RGB",
> [HDMI_COLORSPACE_YUV420]= "YUV 4:2:0",
> @@ -1708,6 +1731,39 @@
> EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property);
> /**
> * DOC: HDMI connector properties
> *
> + * Broadcast RGB (HDMI specific)
> + * Indicates the Quantization Range (Full vs Limited) used. The color
> + * processing pipeline will be adjusted to match the value of the
> + * property, and the Infoframes will be generated and sent accordingly.
> + *
> + * This property is only relevant if the HDMI output format is RGB. If
> + * it's one of the YCbCr variant, it will be ignored and the output will
> + * use a limited quantization range.
> + *
> + * The CRTC attached to the connector must be configured by user-space
> to
> + * always produce full-range pixels.
> + *
> + * The value of this property can be one of the following:
> + *
> + * Automatic:
> + * The quantization range is selected automatically based on the
> + * mode according to the HDMI specifications (HDMI 1.4b -
> Section
> + * 6.6 - Video Quantization Ranges).
> + *
> + * Full:
> + * Full quantization range is forced.
> + *
> + * Limited 16:235:
> + * Limited quantization range is forced. Unlike the name
> suggests,
> + * this works for any number of bits-per-component.
> + *
> + * Property values other than Automatic can result in colors being off
> (if
> + * limited is selected but the display expects full), or a black screen
> + * (if full is selected but the display expects limited).
> + *
> + * Drivers can set up this property by calling
> + * drm_connector_attach_broadcast_rgb_property().
> + *
> * content type (HDMI specific):
> * Indicates content type setting to be used in HDMI infoframes to indicate
> * content type for the external device, so that it adjusts its display
This piece of doc looks good to me, so
Acked-by: Pekka Paalanen
Thanks,
pq
pgp21dou4D3Xl.pgp
Description: OpenPGP digital signature
