diff mbox series

[v4,2/5] drm/drm_property: require DRM_MODE_PROP_IMMUTABLE for single-value props

Message ID 20240715-drm-bridge-connector-fix-hdmi-reset-v4-2-61e6417cfd99@linaro.org (mailing list archive)
State Superseded
Headers show
Series drm: fix two issues related to HDMI Connector implementation | expand

Commit Message

Dmitry Baryshkov July 15, 2024, 6:33 a.m. UTC
Document that DRM_MODE_PROP_IMMUTABLE must be set for the properties
that are immutable by definition - e.g. ranges with min == max or enums
with a single value. This matches the behaviour of the IGT tests, see
kms_properties.c / validate_range_prop(), validate_enum_prop(),
validate_bitmask_prop().

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 include/drm/drm_property.h | 3 +++
 1 file changed, 3 insertions(+)

Comments

Maxime Ripard July 17, 2024, 1:42 p.m. UTC | #1
Hi,

On Mon, Jul 15, 2024 at 09:33:02AM GMT, Dmitry Baryshkov wrote:
> Document that DRM_MODE_PROP_IMMUTABLE must be set for the properties
> that are immutable by definition - e.g. ranges with min == max or enums
> with a single value. This matches the behaviour of the IGT tests, see
> kms_properties.c / validate_range_prop(), validate_enum_prop(),
> validate_bitmask_prop().
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

We had a discussion yesterday about it on IRC with Sima, Simon and
Xaver.

https://oftc.irclog.whitequark.org/dri-devel/2024-07-16#33374622;

The conclusion was that it would create an inconsistency between drivers
on whether a given property is immutable or not, which will lead to more
troubles for userspace.

It's not clear why Ville added that check in the first place, so the
best course of action is to remove the IGT test and get the discussion
started there.

Maxime
Dmitry Baryshkov July 17, 2024, 1:57 p.m. UTC | #2
On Wed, Jul 17, 2024 at 03:42:46PM GMT, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Jul 15, 2024 at 09:33:02AM GMT, Dmitry Baryshkov wrote:
> > Document that DRM_MODE_PROP_IMMUTABLE must be set for the properties
> > that are immutable by definition - e.g. ranges with min == max or enums
> > with a single value. This matches the behaviour of the IGT tests, see
> > kms_properties.c / validate_range_prop(), validate_enum_prop(),
> > validate_bitmask_prop().
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
> We had a discussion yesterday about it on IRC with Sima, Simon and
> Xaver.
> 
> https://oftc.irclog.whitequark.org/dri-devel/2024-07-16#33374622;
> 
> The conclusion was that it would create an inconsistency between drivers
> on whether a given property is immutable or not, which will lead to more
> troubles for userspace.
> 
> It's not clear why Ville added that check in the first place, so the
> best course of action is to remove the IGT test and get the discussion
> started there.

Ack, I'll work on removing those tests later today.
diff mbox series

Patch

diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
index 082f29156b3e..d78ec42de42f 100644
--- a/include/drm/drm_property.h
+++ b/include/drm/drm_property.h
@@ -162,6 +162,9 @@  struct drm_property {
 	 *     userspace, e.g. the EDID, or the connector path property on DP
 	 *     MST sinks. Kernel can update the value of an immutable property
 	 *     by calling drm_object_property_set_value().
+	 *     This flag MUST be set for all properties that have only a
+	 *     single value (e.g. min == max or if enum has only a single
+	 *     value).
 	 */
 	uint32_t flags;