diff mbox

[1/3] drm/i915: don't set modes for 2 connectors on the same encoder

Message ID 1389113755-2021-1-git-send-email-przanoni@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulo Zanoni Jan. 7, 2014, 4:55 p.m. UTC 
From: Paulo Zanoni <paulo.r.zanoni@intel.com>

In some cases we have more than 1 connector associated to an encoder
(e.g., SDVO, Haswell DP/HDMI) and we can only set a mode for one of
these connectors. If we only allowed modesets for connected connectors
we would never need this patch, but since we do allow modeset for
disconnected connectors we may see user space trying to set modes on
the two connectors attached to the same encoder, so we need to forbid
that.

This problem can be reproduced by running the following
intel-gpu-tools test case:
  ./kms_setmode --run-subtest clone-exclusive-crtc

Thanks to Daniel Vetter for providing a version of this patch on
pastebin.

Credits-to: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

Lespiau, Damien Jan. 10, 2014, 4:17 p.m. UTC | #1 
On Tue, Jan 07, 2014 at 02:55:53PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> In some cases we have more than 1 connector associated to an encoder
> (e.g., SDVO, Haswell DP/HDMI) and we can only set a mode for one of
> these connectors. If we only allowed modesets for connected connectors
> we would never need this patch, but since we do allow modeset for
> disconnected connectors we may see user space trying to set modes on
> the two connectors attached to the same encoder, so we need to forbid
> that.
> 
> This problem can be reproduced by running the following
> intel-gpu-tools test case:
>   ./kms_setmode --run-subtest clone-exclusive-crtc
> 
> Thanks to Daniel Vetter for providing a version of this patch on
> pastebin.
> 
> Credits-to: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

The comment telling what the block does could use a little update, in
any case:

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

I can't review patch 3/3 as I wrote a part of it, we need to find a
victim.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a562eef..7cc67b3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9922,17 +9922,21 @@  intel_modeset_stage_output_state(struct drm_device *dev,
 	/* Check for any encoders that needs to be disabled. */
 	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
 			    base.head) {
+		int num_connectors = 0;
 		list_for_each_entry(connector,
 				    &dev->mode_config.connector_list,
 				    base.head) {
 			if (connector->new_encoder == encoder) {
 				WARN_ON(!connector->new_encoder->new_crtc);
-
-				goto next_encoder;
+				num_connectors++;
 			}
 		}
-		encoder->new_crtc = NULL;
-next_encoder:
+
+		if (num_connectors == 0)
+			encoder->new_crtc = NULL;
+		else if (num_connectors > 1)
+			return -EINVAL;
+
 		/* Only now check for crtc changes so we don't miss encoders
 		 * that will be disabled. */
 		if (&encoder->new_crtc->base != encoder->base.crtc) {