All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND v2 1/2] drm/i915: Add connector property to limit max bpc
@ 2017-11-09  0:31 Radhakrishna Sripada
  2017-11-09  0:31 ` [RESEND v2 2/2] drm/i915: Allow "max bpc" property to limit pipe_bpp Radhakrishna Sripada
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Radhakrishna Sripada @ 2017-11-09  0:31 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

From: "Sripada, Radhakrishna" <radhakrishna.sripada@intel.com>

At times 12bpc HDMI cannot be driven due to faulty cables, dongles
level shifters etc. To workaround them we may need to drive the output
at a lower bpc. Currently the user space does not have a way to limit
the bpc. The default bpc to be programmed is decided by the driver and
is run against connector limitations.

Creating a new connector property "max bpc" in order to limit the bpc
with which the pixels are scanned out. xrandr can make use of this
connector property to make sure that bpc does not exceed the configured value.

V2: Initialize max_bpc to satisfy kms_properties

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_atomic.c |  7 +++++++
 drivers/gpu/drm/i915/intel_drv.h    |  2 ++
 drivers/gpu/drm/i915/intel_hdmi.c   | 11 +++++++++++
 drivers/gpu/drm/i915/intel_modes.c  | 19 +++++++++++++++++++
 5 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fe93115c4caa..9642bc4b7a0b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2477,6 +2477,7 @@ struct drm_i915_private {
 
 	struct drm_property *broadcast_rgb_property;
 	struct drm_property *force_audio_property;
+	struct drm_property *max_bpc_property;
 
 	/* hda/i915 audio component */
 	struct i915_audio_component *audio_component;
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 36d4e635e4ce..d6d836ea1a7f 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -58,6 +58,8 @@ int intel_digital_connector_atomic_get_property(struct drm_connector *connector,
 		*val = intel_conn_state->force_audio;
 	else if (property == dev_priv->broadcast_rgb_property)
 		*val = intel_conn_state->broadcast_rgb;
+	else if (property == dev_priv->max_bpc_property)
+		*val = intel_conn_state->max_bpc;
 	else {
 		DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name);
 		return -EINVAL;
@@ -95,6 +97,11 @@ int intel_digital_connector_atomic_set_property(struct drm_connector *connector,
 		return 0;
 	}
 
+	if (property == dev_priv->max_bpc_property) {
+		intel_conn_state->max_bpc = val;
+		return 0;
+	}
+
 	DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name);
 	return -EINVAL;
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 00b488688042..9004a1ee1e99 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -340,6 +340,7 @@ struct intel_digital_connector_state {
 
 	enum hdmi_force_audio force_audio;
 	int broadcast_rgb;
+	int max_bpc;
 };
 
 #define to_intel_digital_connector_state(x) container_of(x, struct intel_digital_connector_state, base)
@@ -1704,6 +1705,7 @@ int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
 void intel_attach_force_audio_property(struct drm_connector *connector);
 void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
 void intel_attach_aspect_ratio_property(struct drm_connector *connector);
+void intel_attach_max_bpc_property(struct drm_connector *connector, int min, int max);
 
 
 /* intel_overlay.c */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index fa1c793a21ef..4a5dda8aa8c3 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1808,10 +1808,21 @@ static const struct drm_encoder_funcs intel_hdmi_enc_funcs = {
 static void
 intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *connector)
 {
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
+
 	intel_attach_force_audio_property(connector);
 	intel_attach_broadcast_rgb_property(connector);
 	intel_attach_aspect_ratio_property(connector);
 	connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+
+	if ((IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
+	    IS_CHERRYVIEW(dev_priv))) {
+		intel_attach_max_bpc_property(connector, 8, 10);
+		to_intel_digital_connector_state(connector->state)->max_bpc = 10;
+	} else if (INTEL_GEN(dev_priv) >= 5) {
+		intel_attach_max_bpc_property(connector, 8, 12);
+		to_intel_digital_connector_state(connector->state)->max_bpc = 12;
+	}
 }
 
 /*
diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
index 4e43f873c889..8357e494e911 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -150,3 +150,22 @@ intel_attach_aspect_ratio_property(struct drm_connector *connector)
 			connector->dev->mode_config.aspect_ratio_property,
 			DRM_MODE_PICTURE_ASPECT_NONE);
 }
+
+void
+intel_attach_max_bpc_property(struct drm_connector *connector, int min, int max)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_property *prop;
+
+	prop = dev_priv->max_bpc_property;
+	if (prop == NULL) {
+		prop = drm_property_create_range(dev, 0, "max bpc", min, max);
+		if (prop == NULL)
+			return;
+
+		dev_priv->max_bpc_property = prop;
+	}
+
+	drm_object_attach_property(&connector->base, prop, max);
+}
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RESEND v2 2/2] drm/i915: Allow "max bpc" property to limit pipe_bpp
  2017-11-09  0:31 [RESEND v2 1/2] drm/i915: Add connector property to limit max bpc Radhakrishna Sripada
@ 2017-11-09  0:31 ` Radhakrishna Sripada
  2017-11-09  1:13 ` ✓ Fi.CI.BAT: success for series starting with [RESEND,v2,1/2] drm/i915: Add connector property to limit max bpc Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Radhakrishna Sripada @ 2017-11-09  0:31 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

From: "Sripada, Radhakrishna" <radhakrishna.sripada@intel.com>

Use the newly added "max bpc" connector property to limit pipe bpp.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 737de251d0f8..4a3359f19b82 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10510,6 +10510,28 @@ connected_sink_compute_bpp(struct intel_connector *connector,
 	}
 }
 
+static void
+connected_sink_max_bpp(struct intel_digital_connector_state *intel_conn_state,
+			     struct intel_crtc_state *pipe_config)
+{
+	switch (intel_conn_state->max_bpc) {
+	case 8:
+	case 9:
+		pipe_config->pipe_bpp = 8*3;
+		break;
+	case 10:
+	case 11:
+		pipe_config->pipe_bpp = 10*3;
+		break;
+	case 12:
+		pipe_config->pipe_bpp = 12*3;
+		break;
+	default:
+		break;
+	}
+	DRM_DEBUG_KMS("Limiting display bpp to %d\n", pipe_config->pipe_bpp);
+}
+
 static int
 compute_baseline_pipe_bpp(struct intel_crtc *crtc,
 			  struct intel_crtc_state *pipe_config)
@@ -10518,6 +10540,7 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
 	struct drm_atomic_state *state;
 	struct drm_connector *connector;
 	struct drm_connector_state *connector_state;
+	struct intel_digital_connector_state *intel_conn_state;
 	int bpp, i;
 
 	if ((IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
@@ -10538,6 +10561,10 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
 		if (connector_state->crtc != &crtc->base)
 			continue;
 
+		intel_conn_state = to_intel_digital_connector_state(connector_state);
+		if (intel_conn_state->max_bpc)
+			connected_sink_max_bpp(intel_conn_state, pipe_config);
+
 		connected_sink_compute_bpp(to_intel_connector(connector),
 					   pipe_config);
 	}
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [RESEND,v2,1/2] drm/i915: Add connector property to limit max bpc
  2017-11-09  0:31 [RESEND v2 1/2] drm/i915: Add connector property to limit max bpc Radhakrishna Sripada
  2017-11-09  0:31 ` [RESEND v2 2/2] drm/i915: Allow "max bpc" property to limit pipe_bpp Radhakrishna Sripada
@ 2017-11-09  1:13 ` Patchwork
  2017-11-09  2:21 ` ✗ Fi.CI.IGT: failure " Patchwork
  2017-11-09 21:24 ` [RESEND v2 1/2] " Rodrigo Vivi
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2017-11-09  1:13 UTC (permalink / raw)
  To: Radhakrishna Sripada; +Cc: intel-gfx

== Series Details ==

Series: series starting with [RESEND,v2,1/2] drm/i915: Add connector property to limit max bpc
URL   : https://patchwork.freedesktop.org/series/33466/
State : success

== Summary ==

Series 33466v1 series starting with [RESEND,v2,1/2] drm/i915: Add connector property to limit max bpc
https://patchwork.freedesktop.org/api/1.0/series/33466/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102514

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:445s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:455s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:379s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:542s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:273s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:497s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:500s
fi-byt-j1900     total:289  pass:254  dwarn:0   dfail:0   fail:0   skip:35  time:493s
fi-byt-n2820     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:484s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:423s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:262s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:542s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:426s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:439s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:427s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:484s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:461s
fi-kbl-7500u     total:289  pass:263  dwarn:1   dfail:0   fail:1   skip:24  time:475s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:525s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:473s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:537s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:572s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:449s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:546s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:563s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:518s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:494s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:456s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:557s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:426s
Blacklisted hosts:
fi-cfl-s         total:289  pass:254  dwarn:3   dfail:0   fail:0   skip:32  time:526s
fi-cnl-y         total:289  pass:215  dwarn:0   dfail:0   fail:0   skip:24 

1446a30948d7c5570e5cfc40678dbdd74e152498 drm-tip: 2017y-11m-08d-21h-46m-48s UTC integration manifest
415e657f3e58 drm/i915: Allow "max bpc" property to limit pipe_bpp
77427c3fe1cd drm/i915: Add connector property to limit max bpc

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7023/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* ✗ Fi.CI.IGT: failure for series starting with [RESEND,v2,1/2] drm/i915: Add connector property to limit max bpc
  2017-11-09  0:31 [RESEND v2 1/2] drm/i915: Add connector property to limit max bpc Radhakrishna Sripada
  2017-11-09  0:31 ` [RESEND v2 2/2] drm/i915: Allow "max bpc" property to limit pipe_bpp Radhakrishna Sripada
  2017-11-09  1:13 ` ✓ Fi.CI.BAT: success for series starting with [RESEND,v2,1/2] drm/i915: Add connector property to limit max bpc Patchwork
@ 2017-11-09  2:21 ` Patchwork
  2017-11-09 21:24 ` [RESEND v2 1/2] " Rodrigo Vivi
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2017-11-09  2:21 UTC (permalink / raw)
  To: Radhakrishna Sripada; +Cc: intel-gfx

== Series Details ==

Series: series starting with [RESEND,v2,1/2] drm/i915: Add connector property to limit max bpc
URL   : https://patchwork.freedesktop.org/series/33466/
State : failure

== Summary ==

Test kms_cursor_legacy:
        Subgroup flip-vs-cursor-varying-size:
                pass       -> FAIL       (shard-hsw)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                skip       -> PASS       (shard-hsw)
Test kms_busy:
        Subgroup extended-modeset-hang-newfb-with-reset-render-a:
                pass       -> DMESG-WARN (shard-hsw) fdo#102249

fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249

shard-hsw        total:2540 pass:1430 dwarn:2   dfail:0   fail:11  skip:1097 time:9265s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7023/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RESEND v2 1/2] drm/i915: Add connector property to limit max bpc
  2017-11-09  0:31 [RESEND v2 1/2] drm/i915: Add connector property to limit max bpc Radhakrishna Sripada
                   ` (2 preceding siblings ...)
  2017-11-09  2:21 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2017-11-09 21:24 ` Rodrigo Vivi
  2017-11-10 19:47   ` Sripada, Radhakrishna
  3 siblings, 1 reply; 7+ messages in thread
From: Rodrigo Vivi @ 2017-11-09 21:24 UTC (permalink / raw)
  To: Radhakrishna Sripada; +Cc: intel-gfx, Paulo Zanoni

On Thu, Nov 09, 2017 at 12:31:51AM +0000, Radhakrishna Sripada wrote:
> From: "Sripada, Radhakrishna" <radhakrishna.sripada@intel.com>
> 
> At times 12bpc HDMI cannot be driven due to faulty cables, dongles
> level shifters etc. To workaround them we may need to drive the output
> at a lower bpc. Currently the user space does not have a way to limit
> the bpc. The default bpc to be programmed is decided by the driver and
> is run against connector limitations.
> 
> Creating a new connector property "max bpc" in order to limit the bpc
> with which the pixels are scanned out. xrandr can make use of this
> connector property to make sure that bpc does not exceed the configured value.

I wonder if this case qualifies to "Linux is not about choice"...

Or all test cases we already have in place would cover all possible
combinations in CI to avoid dealing with future regressions.

> 
> V2: Initialize max_bpc to satisfy kms_properties
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_atomic.c |  7 +++++++
>  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
>  drivers/gpu/drm/i915/intel_hdmi.c   | 11 +++++++++++
>  drivers/gpu/drm/i915/intel_modes.c  | 19 +++++++++++++++++++
>  5 files changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index fe93115c4caa..9642bc4b7a0b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2477,6 +2477,7 @@ struct drm_i915_private {
>  
>  	struct drm_property *broadcast_rgb_property;
>  	struct drm_property *force_audio_property;
> +	struct drm_property *max_bpc_property;
>  
>  	/* hda/i915 audio component */
>  	struct i915_audio_component *audio_component;
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 36d4e635e4ce..d6d836ea1a7f 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -58,6 +58,8 @@ int intel_digital_connector_atomic_get_property(struct drm_connector *connector,
>  		*val = intel_conn_state->force_audio;
>  	else if (property == dev_priv->broadcast_rgb_property)
>  		*val = intel_conn_state->broadcast_rgb;
> +	else if (property == dev_priv->max_bpc_property)
> +		*val = intel_conn_state->max_bpc;
>  	else {
>  		DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name);
>  		return -EINVAL;
> @@ -95,6 +97,11 @@ int intel_digital_connector_atomic_set_property(struct drm_connector *connector,
>  		return 0;
>  	}
>  
> +	if (property == dev_priv->max_bpc_property) {
> +		intel_conn_state->max_bpc = val;
> +		return 0;
> +	}
> +
>  	DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name);
>  	return -EINVAL;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 00b488688042..9004a1ee1e99 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -340,6 +340,7 @@ struct intel_digital_connector_state {
>  
>  	enum hdmi_force_audio force_audio;
>  	int broadcast_rgb;
> +	int max_bpc;
>  };
>  
>  #define to_intel_digital_connector_state(x) container_of(x, struct intel_digital_connector_state, base)
> @@ -1704,6 +1705,7 @@ int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
>  void intel_attach_force_audio_property(struct drm_connector *connector);
>  void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
>  void intel_attach_aspect_ratio_property(struct drm_connector *connector);
> +void intel_attach_max_bpc_property(struct drm_connector *connector, int min, int max);
>  
>  
>  /* intel_overlay.c */
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index fa1c793a21ef..4a5dda8aa8c3 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1808,10 +1808,21 @@ static const struct drm_encoder_funcs intel_hdmi_enc_funcs = {
>  static void
>  intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *connector)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> +
>  	intel_attach_force_audio_property(connector);
>  	intel_attach_broadcast_rgb_property(connector);
>  	intel_attach_aspect_ratio_property(connector);
>  	connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> +
> +	if ((IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
> +	    IS_CHERRYVIEW(dev_priv))) {
> +		intel_attach_max_bpc_property(connector, 8, 10);
> +		to_intel_digital_connector_state(connector->state)->max_bpc = 10;
> +	} else if (INTEL_GEN(dev_priv) >= 5) {
> +		intel_attach_max_bpc_property(connector, 8, 12);
> +		to_intel_digital_connector_state(connector->state)->max_bpc = 12;
> +	}
>  }
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
> index 4e43f873c889..8357e494e911 100644
> --- a/drivers/gpu/drm/i915/intel_modes.c
> +++ b/drivers/gpu/drm/i915/intel_modes.c
> @@ -150,3 +150,22 @@ intel_attach_aspect_ratio_property(struct drm_connector *connector)
>  			connector->dev->mode_config.aspect_ratio_property,
>  			DRM_MODE_PICTURE_ASPECT_NONE);
>  }
> +
> +void
> +intel_attach_max_bpc_property(struct drm_connector *connector, int min, int max)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_property *prop;
> +
> +	prop = dev_priv->max_bpc_property;
> +	if (prop == NULL) {
> +		prop = drm_property_create_range(dev, 0, "max bpc", min, max);
> +		if (prop == NULL)
> +			return;
> +
> +		dev_priv->max_bpc_property = prop;
> +	}
> +
> +	drm_object_attach_property(&connector->base, prop, max);
> +}
> -- 
> 2.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RESEND v2 1/2] drm/i915: Add connector property to limit max bpc
  2017-11-09 21:24 ` [RESEND v2 1/2] " Rodrigo Vivi
@ 2017-11-10 19:47   ` Sripada, Radhakrishna
  2017-11-13 18:53     ` Rodrigo Vivi
  0 siblings, 1 reply; 7+ messages in thread
From: Sripada, Radhakrishna @ 2017-11-10 19:47 UTC (permalink / raw)
  To: Vivi, Rodrigo, Ville Syrjälä; +Cc: intel-gfx, Zanoni, Paulo R



> -----Original Message-----
> From: Vivi, Rodrigo
> Sent: Thursday, November 9, 2017 1:25 PM
> To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Zanoni, Paulo R
> <paulo.r.zanoni@intel.com>
> Subject: Re: [Intel-gfx] [RESEND v2 1/2] drm/i915: Add connector property to
> limit max bpc
> 
> On Thu, Nov 09, 2017 at 12:31:51AM +0000, Radhakrishna Sripada wrote:
> > From: "Sripada, Radhakrishna" <radhakrishna.sripada@intel.com>
> >
> > At times 12bpc HDMI cannot be driven due to faulty cables, dongles
> > level shifters etc. To workaround them we may need to drive the output
> > at a lower bpc. Currently the user space does not have a way to limit
> > the bpc. The default bpc to be programmed is decided by the driver and
> > is run against connector limitations.
> >
> > Creating a new connector property "max bpc" in order to limit the bpc
> > with which the pixels are scanned out. xrandr can make use of this
> > connector property to make sure that bpc does not exceed the configured
> value.
> 
> I wonder if this case qualifies to "Linux is not about choice"...
When we are dealing with faulty hardware like cables/dongles/level-shifters we would need a quirk
to try falling back to a lower bpc and drive the output.  However we set the default to the maximum 
possible value that the driver is anyway currently doing. Is there an alternative that you would suggest 
in this case?

> 
> Or all test cases we already have in place would cover all possible
> combinations in CI to avoid dealing with future regressions.
> 
> >
> > V2: Initialize max_bpc to satisfy kms_properties
> >
> > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h     |  1 +
> >  drivers/gpu/drm/i915/intel_atomic.c |  7 +++++++
> >  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
> >  drivers/gpu/drm/i915/intel_hdmi.c   | 11 +++++++++++
> >  drivers/gpu/drm/i915/intel_modes.c  | 19 +++++++++++++++++++
> >  5 files changed, 40 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> > index fe93115c4caa..9642bc4b7a0b 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2477,6 +2477,7 @@ struct drm_i915_private {
> >
> >  	struct drm_property *broadcast_rgb_property;
> >  	struct drm_property *force_audio_property;
> > +	struct drm_property *max_bpc_property;
> >
> >  	/* hda/i915 audio component */
> >  	struct i915_audio_component *audio_component;
> > diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> b/drivers/gpu/drm/i915/intel_atomic.c
> > index 36d4e635e4ce..d6d836ea1a7f 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > @@ -58,6 +58,8 @@ int
> intel_digital_connector_atomic_get_property(struct drm_connector
> *connector,
> >  		*val = intel_conn_state->force_audio;
> >  	else if (property == dev_priv->broadcast_rgb_property)
> >  		*val = intel_conn_state->broadcast_rgb;
> > +	else if (property == dev_priv->max_bpc_property)
> > +		*val = intel_conn_state->max_bpc;
> >  	else {
> >  		DRM_DEBUG_ATOMIC("Unknown property %s\n",
> property->name);
> >  		return -EINVAL;
> > @@ -95,6 +97,11 @@ int
> intel_digital_connector_atomic_set_property(struct drm_connector
> *connector,
> >  		return 0;
> >  	}
> >
> > +	if (property == dev_priv->max_bpc_property) {
> > +		intel_conn_state->max_bpc = val;
> > +		return 0;
> > +	}
> > +
> >  	DRM_DEBUG_ATOMIC("Unknown property %s\n", property-
> >name);
> >  	return -EINVAL;
> >  }
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> > index 00b488688042..9004a1ee1e99 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -340,6 +340,7 @@ struct intel_digital_connector_state {
> >
> >  	enum hdmi_force_audio force_audio;
> >  	int broadcast_rgb;
> > +	int max_bpc;
> >  };
> >
> >  #define to_intel_digital_connector_state(x) container_of(x, struct
> intel_digital_connector_state, base)
> > @@ -1704,6 +1705,7 @@ int intel_ddc_get_modes(struct drm_connector
> *c, struct i2c_adapter *adapter);
> >  void intel_attach_force_audio_property(struct drm_connector
> *connector);
> >  void intel_attach_broadcast_rgb_property(struct drm_connector
> *connector);
> >  void intel_attach_aspect_ratio_property(struct drm_connector
> *connector);
> > +void intel_attach_max_bpc_property(struct drm_connector *connector,
> int min, int max);
> >
> >
> >  /* intel_overlay.c */
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> > index fa1c793a21ef..4a5dda8aa8c3 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1808,10 +1808,21 @@ static const struct drm_encoder_funcs
> intel_hdmi_enc_funcs = {
> >  static void
> >  intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct
> drm_connector *connector)
> >  {
> > +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > +
> >  	intel_attach_force_audio_property(connector);
> >  	intel_attach_broadcast_rgb_property(connector);
> >  	intel_attach_aspect_ratio_property(connector);
> >  	connector->state->picture_aspect_ratio =
> HDMI_PICTURE_ASPECT_NONE;
> > +
> > +	if ((IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
> > +	    IS_CHERRYVIEW(dev_priv))) {
> > +		intel_attach_max_bpc_property(connector, 8, 10);
> > +		to_intel_digital_connector_state(connector->state)-
> >max_bpc = 10;
> > +	} else if (INTEL_GEN(dev_priv) >= 5) {
> > +		intel_attach_max_bpc_property(connector, 8, 12);
> > +		to_intel_digital_connector_state(connector->state)-
> >max_bpc = 12;
> > +	}
> >  }
> >
> >  /*
> > diff --git a/drivers/gpu/drm/i915/intel_modes.c
> b/drivers/gpu/drm/i915/intel_modes.c
> > index 4e43f873c889..8357e494e911 100644
> > --- a/drivers/gpu/drm/i915/intel_modes.c
> > +++ b/drivers/gpu/drm/i915/intel_modes.c
> > @@ -150,3 +150,22 @@ intel_attach_aspect_ratio_property(struct
> drm_connector *connector)
> >  			connector->dev-
> >mode_config.aspect_ratio_property,
> >  			DRM_MODE_PICTURE_ASPECT_NONE);
> >  }
> > +
> > +void
> > +intel_attach_max_bpc_property(struct drm_connector *connector, int
> min, int max)
> > +{
> > +	struct drm_device *dev = connector->dev;
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> > +	struct drm_property *prop;
> > +
> > +	prop = dev_priv->max_bpc_property;
> > +	if (prop == NULL) {
> > +		prop = drm_property_create_range(dev, 0, "max bpc", min,
> max);
> > +		if (prop == NULL)
> > +			return;
> > +
> > +		dev_priv->max_bpc_property = prop;
> > +	}
> > +
> > +	drm_object_attach_property(&connector->base, prop, max);
> > +}
> > --
> > 2.9.3
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RESEND v2 1/2] drm/i915: Add connector property to limit max bpc
  2017-11-10 19:47   ` Sripada, Radhakrishna
@ 2017-11-13 18:53     ` Rodrigo Vivi
  0 siblings, 0 replies; 7+ messages in thread
From: Rodrigo Vivi @ 2017-11-13 18:53 UTC (permalink / raw)
  To: Sripada, Radhakrishna; +Cc: intel-gfx, Zanoni, Paulo R

On Fri, Nov 10, 2017 at 07:47:46PM +0000, Sripada, Radhakrishna wrote:
> 
> 
> > -----Original Message-----
> > From: Vivi, Rodrigo
> > Sent: Thursday, November 9, 2017 1:25 PM
> > To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; Zanoni, Paulo R
> > <paulo.r.zanoni@intel.com>
> > Subject: Re: [Intel-gfx] [RESEND v2 1/2] drm/i915: Add connector property to
> > limit max bpc
> > 
> > On Thu, Nov 09, 2017 at 12:31:51AM +0000, Radhakrishna Sripada wrote:
> > > From: "Sripada, Radhakrishna" <radhakrishna.sripada@intel.com>
> > >
> > > At times 12bpc HDMI cannot be driven due to faulty cables, dongles
> > > level shifters etc. To workaround them we may need to drive the output
> > > at a lower bpc. Currently the user space does not have a way to limit
> > > the bpc. The default bpc to be programmed is decided by the driver and
> > > is run against connector limitations.
> > >
> > > Creating a new connector property "max bpc" in order to limit the bpc
> > > with which the pixels are scanned out. xrandr can make use of this
> > > connector property to make sure that bpc does not exceed the configured
> > value.
> > 
> > I wonder if this case qualifies to "Linux is not about choice"...
> When we are dealing with faulty hardware like cables/dongles/level-shifters we would need a quirk
> to try falling back to a lower bpc and drive the output.  However we set the default to the maximum 
> possible value that the driver is anyway currently doing. Is there an alternative that you would suggest 
> in this case?

We deal with so may faulty stuff already... But it is "better" when we do in a
broader way without exposing an stable api that we have to maintain forever
because few bad cables and dongles out there.

> 
> > 
> > Or all test cases we already have in place would cover all possible
> > combinations in CI to avoid dealing with future regressions.
> > 
> > >
> > > V2: Initialize max_bpc to satisfy kms_properties
> > >
> > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h     |  1 +
> > >  drivers/gpu/drm/i915/intel_atomic.c |  7 +++++++
> > >  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
> > >  drivers/gpu/drm/i915/intel_hdmi.c   | 11 +++++++++++
> > >  drivers/gpu/drm/i915/intel_modes.c  | 19 +++++++++++++++++++
> > >  5 files changed, 40 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > > index fe93115c4caa..9642bc4b7a0b 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2477,6 +2477,7 @@ struct drm_i915_private {
> > >
> > >  	struct drm_property *broadcast_rgb_property;
> > >  	struct drm_property *force_audio_property;
> > > +	struct drm_property *max_bpc_property;
> > >
> > >  	/* hda/i915 audio component */
> > >  	struct i915_audio_component *audio_component;
> > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> > b/drivers/gpu/drm/i915/intel_atomic.c
> > > index 36d4e635e4ce..d6d836ea1a7f 100644
> > > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > > @@ -58,6 +58,8 @@ int
> > intel_digital_connector_atomic_get_property(struct drm_connector
> > *connector,
> > >  		*val = intel_conn_state->force_audio;
> > >  	else if (property == dev_priv->broadcast_rgb_property)
> > >  		*val = intel_conn_state->broadcast_rgb;
> > > +	else if (property == dev_priv->max_bpc_property)
> > > +		*val = intel_conn_state->max_bpc;
> > >  	else {
> > >  		DRM_DEBUG_ATOMIC("Unknown property %s\n",
> > property->name);
> > >  		return -EINVAL;
> > > @@ -95,6 +97,11 @@ int
> > intel_digital_connector_atomic_set_property(struct drm_connector
> > *connector,
> > >  		return 0;
> > >  	}
> > >
> > > +	if (property == dev_priv->max_bpc_property) {
> > > +		intel_conn_state->max_bpc = val;
> > > +		return 0;
> > > +	}
> > > +
> > >  	DRM_DEBUG_ATOMIC("Unknown property %s\n", property-
> > >name);
> > >  	return -EINVAL;
> > >  }
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > > index 00b488688042..9004a1ee1e99 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -340,6 +340,7 @@ struct intel_digital_connector_state {
> > >
> > >  	enum hdmi_force_audio force_audio;
> > >  	int broadcast_rgb;
> > > +	int max_bpc;
> > >  };
> > >
> > >  #define to_intel_digital_connector_state(x) container_of(x, struct
> > intel_digital_connector_state, base)
> > > @@ -1704,6 +1705,7 @@ int intel_ddc_get_modes(struct drm_connector
> > *c, struct i2c_adapter *adapter);
> > >  void intel_attach_force_audio_property(struct drm_connector
> > *connector);
> > >  void intel_attach_broadcast_rgb_property(struct drm_connector
> > *connector);
> > >  void intel_attach_aspect_ratio_property(struct drm_connector
> > *connector);
> > > +void intel_attach_max_bpc_property(struct drm_connector *connector,
> > int min, int max);
> > >
> > >
> > >  /* intel_overlay.c */
> > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > b/drivers/gpu/drm/i915/intel_hdmi.c
> > > index fa1c793a21ef..4a5dda8aa8c3 100644
> > > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > > @@ -1808,10 +1808,21 @@ static const struct drm_encoder_funcs
> > intel_hdmi_enc_funcs = {
> > >  static void
> > >  intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct
> > drm_connector *connector)
> > >  {
> > > +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > > +
> > >  	intel_attach_force_audio_property(connector);
> > >  	intel_attach_broadcast_rgb_property(connector);
> > >  	intel_attach_aspect_ratio_property(connector);
> > >  	connector->state->picture_aspect_ratio =
> > HDMI_PICTURE_ASPECT_NONE;
> > > +
> > > +	if ((IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
> > > +	    IS_CHERRYVIEW(dev_priv))) {
> > > +		intel_attach_max_bpc_property(connector, 8, 10);
> > > +		to_intel_digital_connector_state(connector->state)-
> > >max_bpc = 10;
> > > +	} else if (INTEL_GEN(dev_priv) >= 5) {
> > > +		intel_attach_max_bpc_property(connector, 8, 12);
> > > +		to_intel_digital_connector_state(connector->state)-
> > >max_bpc = 12;
> > > +	}
> > >  }
> > >
> > >  /*
> > > diff --git a/drivers/gpu/drm/i915/intel_modes.c
> > b/drivers/gpu/drm/i915/intel_modes.c
> > > index 4e43f873c889..8357e494e911 100644
> > > --- a/drivers/gpu/drm/i915/intel_modes.c
> > > +++ b/drivers/gpu/drm/i915/intel_modes.c
> > > @@ -150,3 +150,22 @@ intel_attach_aspect_ratio_property(struct
> > drm_connector *connector)
> > >  			connector->dev-
> > >mode_config.aspect_ratio_property,
> > >  			DRM_MODE_PICTURE_ASPECT_NONE);
> > >  }
> > > +
> > > +void
> > > +intel_attach_max_bpc_property(struct drm_connector *connector, int
> > min, int max)
> > > +{
> > > +	struct drm_device *dev = connector->dev;
> > > +	struct drm_i915_private *dev_priv = to_i915(dev);
> > > +	struct drm_property *prop;
> > > +
> > > +	prop = dev_priv->max_bpc_property;
> > > +	if (prop == NULL) {
> > > +		prop = drm_property_create_range(dev, 0, "max bpc", min,
> > max);
> > > +		if (prop == NULL)
> > > +			return;
> > > +
> > > +		dev_priv->max_bpc_property = prop;
> > > +	}
> > > +
> > > +	drm_object_attach_property(&connector->base, prop, max);
> > > +}
> > > --
> > > 2.9.3
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-11-13 18:53 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-09  0:31 [RESEND v2 1/2] drm/i915: Add connector property to limit max bpc Radhakrishna Sripada
2017-11-09  0:31 ` [RESEND v2 2/2] drm/i915: Allow "max bpc" property to limit pipe_bpp Radhakrishna Sripada
2017-11-09  1:13 ` ✓ Fi.CI.BAT: success for series starting with [RESEND,v2,1/2] drm/i915: Add connector property to limit max bpc Patchwork
2017-11-09  2:21 ` ✗ Fi.CI.IGT: failure " Patchwork
2017-11-09 21:24 ` [RESEND v2 1/2] " Rodrigo Vivi
2017-11-10 19:47   ` Sripada, Radhakrishna
2017-11-13 18:53     ` Rodrigo Vivi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.