Re: The state of Quantization Range handling
Hello Sebastian, I've previously done some work on this topic [1]. My efforts were mostly about fixing the situation regarding overrides and providing proper means for userspace. I am affected by the issue myself as I own several DELL U2414H screens that declare a CE mode as their preferred one, but should receive full range data nonetheless. They do not have the respective bit set in their EDID to indicate full range support either. My implementation primarily moved the "Broadcast RGB" to DRM core and re-wired it in i915 and gma500. I further added a flag to indicate CE modes to userspace so that apps such as gnome-control-center can clearly communicate whether full or limited range would be used by default. A v2 branch that I never submitted is available at [2]. I also have some code lying around locally that adds the required functionality to mutter and gnome-control-center. I had to pause work on the issue back then and never really came around to picking it up again, however, I would be interested in working on it again if there is consensus on the direction that my patches laid out. I did not consider use cases for the out-of-range bits though. Regards Yussuf [1] https://patchwork.kernel.org/project/dri-devel/cover/20200413214024.46500-1-...@pp3345.net/ [2] https://github.com/pp3345/linux/commits/rgb-quant-range-v2 On 15.11.22 00:11, Sebastian Wick wrote: > There are still regular bug reports about monitors (sinks) and sources > disagreeing about the quantization range of the pixel data. In > particular sources sending full range data when the sink expects > limited range. From a user space perspective, this is all hidden in > the kernel. We send full range data to the kernel and then hope it > does the right thing but as the bug reports show: some combinations of > displays and drivers result in problems. > > In general the whole handling of the quantization range on linux is > not defined or documented at all. User space sends full range data > because that's what seems to work most of the time but technically > this is all undefined and user space can not fix those issues. Some > compositors have resorted to giving users the option to choose the > quantization range but this really should only be necessary for > straight up broken hardware. > > Quantization Range can be explicitly controlled by AVI InfoFrame or > HDMI General Control Packets. This is the ideal case and when the > source uses them there is not a lot that can go wrong. Not all > displays support those explicit controls in which case the chosen > video format (IT, CE, SD; details in CTA-861-H 5.1) influences which > quantization range the sink expects. > > This means that we have to expect that sometimes we have to send > limited and sometimes full range content. The big question however > that is not answered in the docs: who is responsible for making sure > the data is in the correct range? Is it the kernel or user space? > > If it's the kernel: does user space supply full range or limited range > content? Each of those has a disadvantage. If we send full range > content and the driver scales it down to limited range, we can't use > the out-of-range bits to transfer information. If we send limited > range content and the driver scales it up we lose information. > > Either way, this must be documented. My suggestion is to say that the > kernel always expects full range data as input and is responsible for > scaling it to limited range data if the sink expects limited range > data. > > Another problem is that some displays do not behave correctly. It must > be possible to override the kernel when the user detects such a > situation. This override then controls if the driver converts the full > range data coming from the client or not (Default, Force Limited, > Force Full). It does not try to control what range the sink expects. > Let's call this the Quantization Range Override property which should > be implemented by all drivers. > > All drivers should make sure their behavior is correct: > > * check that drivers choose the correct default quantization range for > the selected mode > * whenever explicit control is available, use it and set the > quantization range to full > * make sure that the hardware converts from full range to limited > range whenever the sink expects limited range > * implement the Quantization Range Override property > > I'm volunteering for the documentation, UAPI and maybe even the drm > core parts if there is willingness to tackle the issue. > > Appendix A: Broadcast RGB property > > A few drivers already implement the Broadcast RGB property to control > the quantization range. However, it is pointless: It can be set to > Auto, Full and Limited when the sink supports explicitly setting the > quantization range. The driver expects full range content and converts > it to limited range content when necessary. Selecting limited range > never makes any sense: the out-of-range bits
Re: [PATCH 1/5] drm/modes: Indicate CEA-861 CE modes to user-space
On Tue, 2020-04-14 at 14:41 +0200, Daniel Vetter wrote:
> On Mon, Apr 13, 2020 at 11:40:22PM +0200, Yussuf Khalil wrote:
> > Add a new flag to mark modes that are considered a CE mode
> > according to the
> > CEA-861 specification. Modes without this flag are implicitly
> > considered to
> > be IT modes.
> >
> > User-space applications may use this flag to determine possible
> > implications of using a CE mode (e.g., limited RGB range).
> >
> > There is no use for this flag inside the kernel, so we set it only
> > when
> > communicating a mode to user-space.
> >
> > Signed-off-by: Yussuf Khalil
>
> Do we have userspace for this?
>
> If we go with the existing quant range property you don't need new
> userspace for the property itself. But this flag here is new uapi, so
> needs userspace per
>
> https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements
>
> Also since this standardizes kms uapi, we need testcases per
>
> https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#testing-requirements-for-userspace-api
>
> Cheers, Daniel
>
> > ---
> > drivers/gpu/drm/drm_modes.c | 14 ++
> > include/uapi/drm/drm_mode.h | 2 ++
> > 2 files changed, 16 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_modes.c
> > b/drivers/gpu/drm/drm_modes.c
> > index d4d64518e11b..0d8a032f437d 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -1973,6 +1973,14 @@ void drm_mode_convert_to_umode(struct
> > drm_mode_modeinfo *out,
> > break;
> > }
> >
> > + if (drm_match_cea_mode(in) > 1) {
> > + /*
> > +* All modes in CTA-861-G Table 1 are CE modes, except
> > 640x480p
> > +* (VIC 1).
> > +*/
> > + out->flags |= DRM_MODE_FLAG_CEA_861_CE_MODE;
> > + }
> > +
> > strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
> > out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
> > }
> > @@ -2045,6 +2053,12 @@ int drm_mode_convert_umode(struct drm_device
> > *dev,
> > return -EINVAL;
> > }
> >
> > + /*
> > +* The CEA-861 CE mode flag is purely informational and
> > intended for
> > +* userspace only.
> > +*/
> > + out->flags &= ~DRM_MODE_FLAG_CEA_861_CE_MODE;
> > +
> > out->status = drm_mode_validate_driver(dev, out);
> > if (out->status != MODE_OK)
> > return -EINVAL;
> > diff --git a/include/uapi/drm/drm_mode.h
> > b/include/uapi/drm/drm_mode.h
> > index 735c8cfdaaa1..5e78b350b2e2 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -124,6 +124,8 @@ extern "C" {
> > #define DRM_MODE_FLAG_PIC_AR_256_135 \
> > (DRM_MODE_PICTURE_ASPECT_256_135<<19)
> >
> > +#define DRM_MODE_FLAG_CEA_861_CE_MODE (1<<23)
> > +
> > #define DRM_MODE_FLAG_ALL (DRM_MODE_FLAG_PHSYNC | \
> > DRM_MODE_FLAG_NHSYNC | \
> > DRM_MODE_FLAG_PVSYNC | \
> > --
> > 2.26.0
> >
Sorry, I wasn't aware DRM had these additional requirements. I do have a user-
space implementation in mutter and gnome-control-center that makes use of the
new property and this flag on my local machine. I'll try to propose the branch
upstream before sending in the next revision of this patchset.
Do I understand it correctly that this will require test cases for both the
property itself and the new flag? I'll write a patch for IGT then.
Regards
Yussuf
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/5] drm/i915: Replace "Broadcast RGB" with "RGB quantization range" property
On Wed, 2020-04-15 at 13:13 +0200, Daniel Vetter wrote: > On Wed, Apr 15, 2020 at 10:33:25AM +0300, Jani Nikula wrote: > > On Tue, 14 Apr 2020, Yussuf Khalil wrote: > > > On Tue, 2020-04-14 at 14:34 +0200, Daniel Vetter wrote: > > > > On Tue, Apr 14, 2020 at 02:21:06PM +0300, Jani Nikula wrote: > > > > > On Tue, 14 Apr 2020, Jani Nikula > > > > > > > > > > wrote: > > > > > > On Mon, 13 Apr 2020, Simon Ser wrote: > > > > > > > On Monday, April 13, 2020 11:40 PM, Yussuf Khalil < > > > > > > > d...@pp3345.net> wrote: > > > > > > > > > > > > > > > DRM now has a globally available "RGB quantization > > > > > > > > range" > > > > > > > > connector > > > > > > > > property. i915's "Broadcast RGB" that fulfils the same > > > > > > > > purpose is now > > > > > > > > considered deprecated, so drop it in favor of the DRM > > > > > > > > property. > > > > > > > > > > > > > > For a UAPI point-of-view, I'm not sure this is fine. Some > > > > > > > user- > > > > > > > space > > > > > > > might depend on this property, dropping it would break > > > > > > > such > > > > > > > user-space. > > > > > > > > > > > > Agreed. > > > > > > > > > > > > > Can we make this property deprecated but still keep it > > > > > > > for > > > > > > > backwards > > > > > > > compatibility? > > > > > > > > > > > > Would be nice to make the i915 specific property an "alias" > > > > > > for > > > > > > the new > > > > > > property, however I'm not sure how you'd make that happen. > > > > > > Otherwise > > > > > > juggling between the two properties is going to be a > > > > > > nightmare. > > > > > > > > > > Ah, the obvious easy choice is to use the property and enum > > > > > names > > > > > already being used by i915 and gma500, and you have no > > > > > problem. > > > > > Perhaps > > > > > they're not the names you'd like, but then looking at the > > > > > total > > > > > lack of > > > > > consistency across property naming makes them fit right in. > > > > > ;) > > > > > > > > Yeah if we don't have contradictory usage across drivers when > > > > modernizing > > > > these properties, then let's just stick with the names already > > > > there. > > > > It's > > > > not pretty, but works better since more userspace/internet > > > > howtos > > > > know how > > > > to use this stuff. > > > > -Daniel > > > > > > Note that i915's "Broadcast RGB" isn't the same as gma500's: i915 > > > has an > > > "Automatic" option, whereas gma500 does not. > > > > Adding "Automatic" option that just defaults to "Full" in gma500 > > does > > not break existing userspace, but allows for extending it to do > > more > > clever things later. > > gma500 could keep it's own property, without the "Automatic" value. > We've > done tricks like this for other properties too. > > > > Also, radeon has a property called > > > "output_csc" that fulfills a similar purpose. Looking at the > > > code, though, it > > > seems that radeon does not adhere to the standard correctly (or I > > > am missing > > > something). > > > > > > An alternative would be to leave the existing driver-specific > > > properties and > > > change them to be pseudo-aliases for the "RGB quantization range" > > > property. > > > This can be done by letting the drivers read from and write to > > > the new property > > > when user-space tries to read or modify the driver's property. > > > This way we could > > > retain full backwards compatibility for all drivers equally. > > > > > > What do you think? > > > > I'm obviously biased and predisposed to avoid adding extra > > complexity to > > i915 when it's not necessary. We'd have *two* connector properties > > for > > the same thing until the end of time, even if one is an alias for > > the > > other. > > Yeah just pick one, and implement the others as aliases. Drivers can > do > the aliases in their atomic_get/set_property functions pretty easily, > atomic properties aren't stored anywhere else than decoded (which was > done > partially to make stuff like this work). > > Coming up a new property name just so that everyone suffers equally > feels > silly. > -Daniel Okay, I understand your point. Leaving gma500 without a proper implementation of the "Automatic" value isn't an option though as that would break the whole purpose of this patchset: having a property that works precisely the same way across all drivers. I'll build a patch that implements standards-compliant behavior for gma500 then, so that it can use the same property as the others. Regards Yussuf ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/5] drm/i915: Replace "Broadcast RGB" with "RGB quantization range" property
On Tue, 2020-04-14 at 14:34 +0200, Daniel Vetter wrote: > On Tue, Apr 14, 2020 at 02:21:06PM +0300, Jani Nikula wrote: > > On Tue, 14 Apr 2020, Jani Nikula > > wrote: > > > On Mon, 13 Apr 2020, Simon Ser wrote: > > > > On Monday, April 13, 2020 11:40 PM, Yussuf Khalil < > > > > d...@pp3345.net> wrote: > > > > > > > > > DRM now has a globally available "RGB quantization range" > > > > > connector > > > > > property. i915's "Broadcast RGB" that fulfils the same > > > > > purpose is now > > > > > considered deprecated, so drop it in favor of the DRM > > > > > property. > > > > > > > > For a UAPI point-of-view, I'm not sure this is fine. Some user- > > > > space > > > > might depend on this property, dropping it would break such > > > > user-space. > > > > > > Agreed. > > > > > > > Can we make this property deprecated but still keep it for > > > > backwards > > > > compatibility? > > > > > > Would be nice to make the i915 specific property an "alias" for > > > the new > > > property, however I'm not sure how you'd make that happen. > > > Otherwise > > > juggling between the two properties is going to be a nightmare. > > > > Ah, the obvious easy choice is to use the property and enum names > > already being used by i915 and gma500, and you have no problem. > > Perhaps > > they're not the names you'd like, but then looking at the total > > lack of > > consistency across property naming makes them fit right in. ;) > > Yeah if we don't have contradictory usage across drivers when > modernizing > these properties, then let's just stick with the names already there. > It's > not pretty, but works better since more userspace/internet howtos > know how > to use this stuff. > -Daniel Note that i915's "Broadcast RGB" isn't the same as gma500's: i915 has an "Automatic" option, whereas gma500 does not. Also, radeon has a property called "output_csc" that fulfills a similar purpose. Looking at the code, though, it seems that radeon does not adhere to the standard correctly (or I am missing something). An alternative would be to leave the existing driver-specific properties and change them to be pseudo-aliases for the "RGB quantization range" property. This can be done by letting the drivers read from and write to the new property when user-space tries to read or modify the driver's property. This way we could retain full backwards compatibility for all drivers equally. What do you think? Regards Yussuf ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/5] drm/atomic-helper: Consider RGB quantization changes to be mode changes
Ensure RGB quantization range changes are applied immediately. Signed-off-by: Yussuf Khalil --- drivers/gpu/drm/drm_atomic_helper.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 85d163f16801..b74e90a2b214 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -686,6 +686,12 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, if (old_connector_state->max_requested_bpc != new_connector_state->max_requested_bpc) new_crtc_state->connectors_changed = true; + + if (drm_connector_state_select_rgb_quantization_range( + old_connector_state, _crtc_state->mode) != + drm_connector_state_select_rgb_quantization_range( + new_connector_state, _crtc_state->mode)) + new_crtc_state->mode_changed = true; } if (funcs->atomic_check) -- 2.26.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/5] drm/modes: Indicate CEA-861 CE modes to user-space
Add a new flag to mark modes that are considered a CE mode according to the
CEA-861 specification. Modes without this flag are implicitly considered to
be IT modes.
User-space applications may use this flag to determine possible
implications of using a CE mode (e.g., limited RGB range).
There is no use for this flag inside the kernel, so we set it only when
communicating a mode to user-space.
Signed-off-by: Yussuf Khalil
---
drivers/gpu/drm/drm_modes.c | 14 ++
include/uapi/drm/drm_mode.h | 2 ++
2 files changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index d4d64518e11b..0d8a032f437d 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1973,6 +1973,14 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo
*out,
break;
}
+ if (drm_match_cea_mode(in) > 1) {
+ /*
+* All modes in CTA-861-G Table 1 are CE modes, except 640x480p
+* (VIC 1).
+*/
+ out->flags |= DRM_MODE_FLAG_CEA_861_CE_MODE;
+ }
+
strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
}
@@ -2045,6 +2053,12 @@ int drm_mode_convert_umode(struct drm_device *dev,
return -EINVAL;
}
+ /*
+* The CEA-861 CE mode flag is purely informational and intended for
+* userspace only.
+*/
+ out->flags &= ~DRM_MODE_FLAG_CEA_861_CE_MODE;
+
out->status = drm_mode_validate_driver(dev, out);
if (out->status != MODE_OK)
return -EINVAL;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 735c8cfdaaa1..5e78b350b2e2 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -124,6 +124,8 @@ extern "C" {
#define DRM_MODE_FLAG_PIC_AR_256_135 \
(DRM_MODE_PICTURE_ASPECT_256_135<<19)
+#define DRM_MODE_FLAG_CEA_861_CE_MODE (1<<23)
+
#define DRM_MODE_FLAG_ALL (DRM_MODE_FLAG_PHSYNC | \
DRM_MODE_FLAG_NHSYNC | \
DRM_MODE_FLAG_PVSYNC | \
--
2.26.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/5] drm: Add drm_connector_state_select_rgb_quantization_range() helper
This helper can be used to determine the appropriate RGB quantization range
based on a connector's "RGB quantization range" property and a mode.
Signed-off-by: Yussuf Khalil
---
drivers/gpu/drm/drm_connector.c | 31 +++
include/drm/drm_connector.h | 4
2 files changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index e60d3b6e5e56..d5a46bbf284e 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2187,6 +2187,37 @@ int drm_connector_set_panel_orientation_with_quirk(
}
EXPORT_SYMBOL(drm_connector_set_panel_orientation_with_quirk);
+/**
+ * drm_connector_state_select_rgb_quantization_range - find RGB quantization
+ * range appropriate for a connector's state and mode
+ *
+ * @state: state of the connector for which to determine the range
+ * @mode: mode for which to determine the range
+ *
+ * For a given connector state (i.e., RGB quantization range property) and a
+ * given mode, determine which RGB quantization range should be used.
+ *
+ * Returns:
+ * A constant from the HDMI quantization range enum.
+ */
+enum hdmi_quantization_range drm_connector_state_select_rgb_quantization_range(
+ const struct drm_connector_state *state,
+const struct drm_display_mode *mode)
+{
+ switch (state->rgb_quantization_range) {
+ default:
+ WARN_ON(1);
+ /* fallthrough */
+ case DRM_MODE_RGB_QUANTIZATION_RANGE_AUTOMATIC:
+ return drm_default_rgb_quant_range(mode);
+ case DRM_MODE_RGB_QUANTIZATION_RANGE_FULL:
+ return HDMI_QUANTIZATION_RANGE_FULL;
+ case DRM_MODE_RGB_QUANTIZATION_RANGE_LIMITED:
+ return HDMI_QUANTIZATION_RANGE_LIMITED;
+ }
+}
+EXPORT_SYMBOL(drm_connector_state_select_rgb_quantization_range);
+
int drm_connector_set_obj_prop(struct drm_mode_object *obj,
struct drm_property *property,
uint64_t value)
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index f605e0fbcc0e..43ce305d882f 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -42,6 +42,7 @@ struct drm_property_blob;
struct drm_printer;
struct edid;
struct i2c_adapter;
+struct drm_display_mode;
enum drm_connector_force {
DRM_FORCE_UNSPECIFIED,
@@ -1621,6 +1622,9 @@ int drm_connector_set_panel_orientation_with_quirk(
int width, int height);
int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
int min, int max);
+enum hdmi_quantization_range drm_connector_state_select_rgb_quantization_range(
+ const struct drm_connector_state *state,
+ const struct drm_display_mode *mode);
/**
* struct drm_tile_group - Tile group metadata
--
2.26.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/5] drm: Add "RGB quantization range" connector property
Add a new "RGB quantization range" property with three possible values:
Automatic, Limited, and Full. User-space may use this property to override
the automatic selection of the RGB range as specified by CTA-861. Drivers
should attach this property to all CTA-861 sinks.
Signed-off-by: Yussuf Khalil
---
drivers/gpu/drm/drm_atomic_uapi.c | 4
drivers/gpu/drm/drm_connector.c | 35 +++
include/drm/drm_connector.h | 23
include/drm/drm_mode_config.h | 7 +++
4 files changed, 69 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index a1e5e262bae2..f12b3a40c6cf 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -766,6 +766,8 @@ static int drm_atomic_connector_set_property(struct
drm_connector *connector,
fence_ptr);
} else if (property == connector->max_bpc_property) {
state->max_requested_bpc = val;
+ } else if (property == config->rgb_quantization_range_property) {
+ state->rgb_quantization_range = val;
} else if (connector->funcs->atomic_set_property) {
return connector->funcs->atomic_set_property(connector,
state, property, val);
@@ -842,6 +844,8 @@ drm_atomic_connector_get_property(struct drm_connector
*connector,
*val = 0;
} else if (property == connector->max_bpc_property) {
*val = state->max_requested_bpc;
+ } else if (property == config->rgb_quantization_range_property) {
+ *val = state->rgb_quantization_range;
} else if (connector->funcs->atomic_get_property) {
return connector->funcs->atomic_get_property(connector,
state, property, val);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 644f0ad10671..e60d3b6e5e56 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -929,6 +929,12 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
{ DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" },
};
+static const struct drm_prop_enum_list rgb_quantization_range_options[] = {
+ { DRM_MODE_RGB_QUANTIZATION_RANGE_AUTOMATIC, "Automatic" },
+ { DRM_MODE_RGB_QUANTIZATION_RANGE_LIMITED, "Limited" },
+ { DRM_MODE_RGB_QUANTIZATION_RANGE_FULL, "Full" },
+};
+
/**
* DOC: standard connector properties
*
@@ -1804,6 +1810,35 @@ int drm_mode_create_dp_colorspace_property(struct
drm_connector *connector)
}
EXPORT_SYMBOL(drm_mode_create_dp_colorspace_property);
+/**
+ * drm_mode_create_rgb_quantization_range_property - create RGB quantization
+ * range property
+ * @dev: device to create the RGB quantization range property on.
+ *
+ * Called by a driver the first time it's needed, must be attached to
connectors
+ * of CEA-861-compliant sinks.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_mode_create_rgb_quantization_range_property(struct drm_device *dev)
+{
+ if (dev->mode_config.rgb_quantization_range_property)
+ return 0;
+
+ dev->mode_config.rgb_quantization_range_property =
+ drm_property_create_enum(dev, DRM_MODE_PROP_ENUM,
+"RGB quantization range",
+rgb_quantization_range_options,
+
ARRAY_SIZE(rgb_quantization_range_options));
+
+ if (!dev->mode_config.rgb_quantization_range_property)
+ return -ENOMEM;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_mode_create_rgb_quantization_range_property);
+
/**
* drm_mode_create_content_type_property - create content type property
* @dev: DRM device
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 19ae6bb5c85b..f605e0fbcc0e 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -303,6 +303,22 @@ struct drm_monitor_range_info {
#define DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT14
#define DRM_MODE_COLORIMETRY_BT601_YCC 15
+/**
+ * enum drm_rgb_quantization_range - rgb_quantization_range property value
+ *
+ * This enum lists the possible options for the rgb_quantization_range
property.
+ *
+ * @DRM_MODE_RGB_QUANTIZATION_RANGE_AUTOMATIC: Let the driver select the
+ * appropriate quantization range.
+ * @DRM_MODE_RGB_QUANTIZATION_RANGE_LIMITED: Force limited range RGB.
+ * @DRM_MODE_RGB_QUANTIZATION_RANGE_FULL: Force full range RGB.
+ */
+enum drm_rgb_quantization_range {
+ DRM_MODE_RGB_QUANTIZATION_RANGE_AUTOMATIC = 0,
+ DRM_MODE_RGB_QUANTIZATION_RANGE_LIMITED = 1,
+ DRM_MODE_RGB_QU
[PATCH 5/5] drm/i915: Replace "Broadcast RGB" with "RGB quantization range" property
DRM now has a globally available "RGB quantization range" connector
property. i915's "Broadcast RGB" that fulfils the same purpose is now
considered deprecated, so drop it in favor of the DRM property.
Signed-off-by: Yussuf Khalil
---
drivers/gpu/drm/i915/display/intel_atomic.c | 8
.../gpu/drm/i915/display/intel_connector.c| 39 ++-
.../gpu/drm/i915/display/intel_connector.h| 2 +-
.../drm/i915/display/intel_display_types.h| 8
drivers/gpu/drm/i915/display/intel_dp.c | 24
drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
drivers/gpu/drm/i915/display/intel_hdmi.c | 19 -
drivers/gpu/drm/i915/display/intel_sdvo.c | 18 -
drivers/gpu/drm/i915/i915_drv.h | 1 -
9 files changed, 34 insertions(+), 87 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
b/drivers/gpu/drm/i915/display/intel_atomic.c
index d043057d2fa0..5dbbd8e8aa5d 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -63,8 +63,6 @@ int intel_digital_connector_atomic_get_property(struct
drm_connector *connector,
if (property == dev_priv->force_audio_property)
*val = intel_conn_state->force_audio;
- else if (property == dev_priv->broadcast_rgb_property)
- *val = intel_conn_state->broadcast_rgb;
else {
drm_dbg_atomic(_priv->drm,
"Unknown property [PROP:%d:%s]\n",
@@ -99,11 +97,6 @@ int intel_digital_connector_atomic_set_property(struct
drm_connector *connector,
return 0;
}
- if (property == dev_priv->broadcast_rgb_property) {
- intel_conn_state->broadcast_rgb = val;
- return 0;
- }
-
drm_dbg_atomic(_priv->drm, "Unknown property [PROP:%d:%s]\n",
property->base.id, property->name);
return -EINVAL;
@@ -145,7 +138,6 @@ int intel_digital_connector_atomic_check(struct
drm_connector *conn,
* up in a modeset.
*/
if (new_conn_state->force_audio != old_conn_state->force_audio ||
- new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb ||
new_conn_state->base.colorspace != old_conn_state->base.colorspace
||
new_conn_state->base.picture_aspect_ratio !=
old_conn_state->base.picture_aspect_ratio ||
new_conn_state->base.content_type !=
old_conn_state->base.content_type ||
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
b/drivers/gpu/drm/i915/display/intel_connector.c
index 903e49659f56..1b6439e3ccaf 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -238,34 +238,6 @@ intel_attach_force_audio_property(struct drm_connector
*connector)
drm_object_attach_property(>base, prop, 0);
}
-static const struct drm_prop_enum_list broadcast_rgb_names[] = {
- { INTEL_BROADCAST_RGB_AUTO, "Automatic" },
- { INTEL_BROADCAST_RGB_FULL, "Full" },
- { INTEL_BROADCAST_RGB_LIMITED, "Limited 16:235" },
-};
-
-void
-intel_attach_broadcast_rgb_property(struct drm_connector *connector)
-{
- struct drm_device *dev = connector->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct drm_property *prop;
-
- prop = dev_priv->broadcast_rgb_property;
- if (prop == NULL) {
- prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM,
- "Broadcast RGB",
- broadcast_rgb_names,
- ARRAY_SIZE(broadcast_rgb_names));
- if (prop == NULL)
- return;
-
- dev_priv->broadcast_rgb_property = prop;
- }
-
- drm_object_attach_property(>base, prop, 0);
-}
-
void
intel_attach_aspect_ratio_property(struct drm_connector *connector)
{
@@ -297,3 +269,14 @@ intel_attach_colorspace_property(struct drm_connector
*connector)
drm_object_attach_property(>base,
connector->colorspace_property, 0);
}
+
+void
+intel_attach_rgb_quantization_range_property(struct drm_connector *connector)
+{
+ struct drm_device *dev = connector->dev;
+
+ if (!drm_mode_create_rgb_quantization_range_property(dev))
+ drm_object_attach_property(>base,
+ dev->mode_config.rgb_quantization_range_property,
+ DRM_MODE_RGB_QUANTIZATION_RANGE_AUTOMATIC);
+}
\ No newline at end of file
diff --git a/drivers/gpu/drm/i915/display/intel_connector.h
b/drivers/gpu/drm/i915/display/intel_connector.h
index 93a7375c8196..ece946915407 100644
--- a/drivers/gpu/d
[PATCH 0/5] Improving the situation regarding RGB quantization ranges
The CTA-861 specification differentiates between IT and CE modes. For the latter, it requires sources to send "limited RGB quantization range", i.e., allowed RGB values are constrained to 16 - 235 (in the 8 bit case). A sink may indicate support for full range RGB (0 - 255) in CE modes through its EDID, allowing the source to override the recommendations set by CTA-861. For computer screens it is usually desirable to have full range RGB output. In reality, though, many displays set a CE mode as preferred in their EDID and leave the "Quantization Range Selectable" bit unset despite having no issues with full range output. Therefore, it makes sense to provide the user with a method of overriding the standards-compliant selection. The current situation in DRM regarding the RGB quantization range is somewhat messy. We have drivers that - behave standards-compliant and provide a custom override property (e.g., i915, gma500) - behave standards-compliant and provide no override (e.g., vc4) - behave standards-compliant and select full range for CE modes if the screen indicates support (e.g., tda998x) - probably don't care at all This series is an effort to fix the situation. It introduces a property for overriding the RGB quantization range that is defined in the DRM core and can be attached to connectors by all drivers, providing a driver-independent way of overriding the defaults to userspace. So far, I've wired up the new property in i915 only. Yussuf Khalil (5): drm/modes: Indicate CEA-861 CE modes to user-space drm: Add "RGB quantization range" connector property drm: Add drm_connector_state_select_rgb_quantization_range() helper drm/atomic-helper: Consider RGB quantization changes to be mode changes drm/i915: Replace "Broadcast RGB" with "RGB quantization range" property drivers/gpu/drm/drm_atomic_helper.c | 6 ++ drivers/gpu/drm/drm_atomic_uapi.c | 4 ++ drivers/gpu/drm/drm_connector.c | 66 +++ drivers/gpu/drm/drm_modes.c | 14 drivers/gpu/drm/i915/display/intel_atomic.c | 8 --- .../gpu/drm/i915/display/intel_connector.c| 39 --- .../gpu/drm/i915/display/intel_connector.h| 2 +- .../drm/i915/display/intel_display_types.h| 8 --- drivers/gpu/drm/i915/display/intel_dp.c | 24 ++- drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- drivers/gpu/drm/i915/display/intel_hdmi.c | 19 ++ drivers/gpu/drm/i915/display/intel_sdvo.c | 18 ++--- drivers/gpu/drm/i915/i915_drv.h | 1 - include/drm/drm_connector.h | 27 include/drm/drm_mode_config.h | 7 ++ include/uapi/drm/drm_mode.h | 2 + 16 files changed, 160 insertions(+), 87 deletions(-) -- 2.26.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
