[PATCH 0/4] drm/915: Relax output cloning restrictions

[PATCH 0/4] drm/915: Relax output cloning restrictions

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The hardware allows a lot more cloning options than what the driver
currently allows. There are certain easy cloning cases like VGA+HDMI
and HDMI+HDMI which can be enabled very easily, so let's just do it.

LVDS+VGA would be another combination that should be generally available,
but unfortunately the way the KMS API deals with the panel's fixed mode
makes things much more complicated. As it stands, I suspect simply
enabling LVDS+VGA cloning would too easily result in a non-working VGA
display. So leave it out for now.

Ville Syrjälä (4):
  drm/i915: Make encoder cloning more flexible
  drm/i915: Don't use HDMI 12bpc when cloning with other encoder types
  drm/i915: Allow HDMI+VGA cloning
  drm/i915: Allow HDMI+HDMI cloning on g4x

 drivers/gpu/drm/i915/intel_crt.c     |  2 +-
 drivers/gpu/drm/i915/intel_ddi.c     |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 53 ++++++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_dp.c      |  2 +-
 drivers/gpu/drm/i915/intel_drv.h     |  6 +---
 drivers/gpu/drm/i915/intel_dsi.c     |  2 +-
 drivers/gpu/drm/i915/intel_dvo.c     |  5 ++--
 drivers/gpu/drm/i915/intel_hdmi.c    | 36 ++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_lvds.c    |  2 +-
 drivers/gpu/drm/i915/intel_sdvo.c    |  2 +-
 drivers/gpu/drm/i915/intel_tv.c      |  3 +-
 11 files changed, 81 insertions(+), 34 deletions(-)

-- 
1.8.3.2

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

[PATCH 1/4] drm/i915: Make encoder cloning more flexible

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently we allow encoders to indicate whether they can be part of a
cloned set with just one flag. That's not flexible enough to describe
the actual hardware capabilities. Instead make it a bitmask of encoder
types with which the current encoder can be cloned.

For now we set the bitmask to allow DVO+DVO and DVO+VGA, which should
match what the old boolean flag allowed. We will add some more cloning
options in the future.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c     |  2 +-
 drivers/gpu/drm/i915/intel_ddi.c     |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 53 ++++++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_dp.c      |  2 +-
 drivers/gpu/drm/i915/intel_drv.h     |  6 +---
 drivers/gpu/drm/i915/intel_dsi.c     |  2 +-
 drivers/gpu/drm/i915/intel_dvo.c     |  5 ++--
 drivers/gpu/drm/i915/intel_hdmi.c    |  2 +-
 drivers/gpu/drm/i915/intel_lvds.c    |  2 +-
 drivers/gpu/drm/i915/intel_sdvo.c    |  2 +-
 drivers/gpu/drm/i915/intel_tv.c      |  3 +-
 11 files changed, 48 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 9864aa1..da37930 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -800,7 +800,7 @@ void intel_crt_init(struct drm_device *dev)
 	intel_connector_attach_encoder(intel_connector, &crt->base);
 
 	crt->base.type = INTEL_OUTPUT_ANALOG;
-	crt->base.cloneable = true;
+	crt->base.cloneable = 1 << INTEL_OUTPUT_DVO;
 	if (IS_I830(dev))
 		crt->base.crtc_mask = (1 << 0);
 	else
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 2643d3b..a6f77b7 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1712,7 +1712,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
 
 	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
 	intel_encoder->crtc_mask =  (1 << 0) | (1 << 1) | (1 << 2);
-	intel_encoder->cloneable = false;
+	intel_encoder->cloneable = 0;
 	intel_encoder->hot_plug = intel_ddi_hot_plug;
 
 	if (init_dp)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6f15627..677543c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8960,23 +8960,47 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
 }
 
-static bool check_encoder_cloning(struct drm_crtc *crtc)
+static bool encoders_cloneable(const struct intel_encoder *a,
+			       const struct intel_encoder *b)
 {
-	int num_encoders = 0;
-	bool uncloneable_encoders = false;
+	/* masks could be asymmetric, so check both ways */
+	return a == b || (a->cloneable & (1 << b->type) &&
+			  b->cloneable & (1 << a->type));
+}
+
+static bool check_single_encoder_cloning(struct intel_crtc *crtc,
+					 struct intel_encoder *encoder)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct intel_encoder *source_encoder;
+
+	list_for_each_entry(source_encoder,
+			    &dev->mode_config.encoder_list, base.head) {
+		if (source_encoder->new_crtc != crtc)
+			continue;
+
+		if (!encoders_cloneable(encoder, source_encoder))
+			return false;
+	}
+
+	return true;
+}
+
+static bool check_encoder_cloning(struct intel_crtc *crtc)
+{
+	struct drm_device *dev = crtc->base.dev;
 	struct intel_encoder *encoder;
 
-	list_for_each_entry(encoder, &crtc->dev->mode_config.encoder_list,
-			    base.head) {
-		if (&encoder->new_crtc->base != crtc)
+	list_for_each_entry(encoder,
+			    &dev->mode_config.encoder_list, base.head) {
+		if (encoder->new_crtc != crtc)
 			continue;
 
-		num_encoders++;
-		if (!encoder->cloneable)
-			uncloneable_encoders = true;
+		if (!check_single_encoder_cloning(crtc, encoder))
+			return false;
 	}
 
-	return !(num_encoders > 1 && uncloneable_encoders);
+	return true;
 }
 
 static struct intel_crtc_config *
@@ -8990,7 +9014,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 	int plane_bpp, ret = -EINVAL;
 	bool retry = true;
 
-	if (!check_encoder_cloning(crtc)) {
+	if (!check_encoder_cloning(to_intel_crtc(crtc))) {
 		DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
 		return ERR_PTR(-EINVAL);
 	}
@@ -10353,12 +10377,7 @@ static int intel_encoder_clones(struct intel_encoder *encoder)
 
 	list_for_each_entry(source_encoder,
 			    &dev->mode_config.encoder_list, base.head) {
-
-		if (encoder == source_encoder)
-			index_mask |= (1 << entry);
-
-		/* Intel hw has only one MUX where enocoders could be cloned. */
-		if (encoder->cloneable && source_encoder->cloneable)
+		if (encoders_cloneable(encoder, source_encoder))
 			index_mask |= (1 << entry);
 
 		entry++;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c512d78..3bc6aad 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3954,7 +3954,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 
 	intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
-	intel_encoder->cloneable = false;
+	intel_encoder->cloneable = 0;
 	intel_encoder->hot_plug = intel_dp_hot_plug;
 
 	if (!intel_dp_init_connector(intel_dig_port, intel_connector)) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a4ffc02..76fd998 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -124,11 +124,7 @@ struct intel_encoder {
 	struct intel_crtc *new_crtc;
 
 	int type;
-	/*
-	 * Intel hw has only one MUX where encoders could be clone, hence a
-	 * simple flag is enough to compute the possible_clones mask.
-	 */
-	bool cloneable;
+	unsigned int cloneable;
 	bool connectors_active;
 	void (*hot_plug)(struct intel_encoder *);
 	bool (*compute_config)(struct intel_encoder *,
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 3ee1db1..36fb7f9 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -604,7 +604,7 @@ bool intel_dsi_init(struct drm_device *dev)
 	intel_encoder->type = INTEL_OUTPUT_DSI;
 	intel_encoder->crtc_mask = (1 << 0); /* XXX */
 
-	intel_encoder->cloneable = false;
+	intel_encoder->cloneable = 0;
 	drm_connector_init(dev, connector, &intel_dsi_connector_funcs,
 			   DRM_MODE_CONNECTOR_DSI);
 
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 86eeb8b..7fe3fee 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -522,14 +522,15 @@ void intel_dvo_init(struct drm_device *dev)
 		intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
 		switch (dvo->type) {
 		case INTEL_DVO_CHIP_TMDS:
-			intel_encoder->cloneable = true;
+			intel_encoder->cloneable = (1 << INTEL_OUTPUT_ANALOG) |
+				(1 << INTEL_OUTPUT_DVO);
 			drm_connector_init(dev, connector,
 					   &intel_dvo_connector_funcs,
 					   DRM_MODE_CONNECTOR_DVII);
 			encoder_type = DRM_MODE_ENCODER_TMDS;
 			break;
 		case INTEL_DVO_CHIP_LVDS:
-			intel_encoder->cloneable = false;
+			intel_encoder->cloneable = 0;
 			drm_connector_init(dev, connector,
 					   &intel_dvo_connector_funcs,
 					   DRM_MODE_CONNECTOR_LVDS);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index aa4641d..393cd30 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1290,7 +1290,7 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
 
 	intel_encoder->type = INTEL_OUTPUT_HDMI;
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
-	intel_encoder->cloneable = false;
+	intel_encoder->cloneable = 0;
 
 	intel_dig_port->port = port;
 	intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index fecff3c..ef5e566 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -963,7 +963,7 @@ void intel_lvds_init(struct drm_device *dev)
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
 	intel_encoder->type = INTEL_OUTPUT_LVDS;
 
-	intel_encoder->cloneable = false;
+	intel_encoder->cloneable = 0;
 	if (HAS_PCH_SPLIT(dev))
 		intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
 	else if (IS_GEN4(dev))
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 825853d..9a0b71f 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -3032,7 +3032,7 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
 	 * simplistic anyway to express such constraints, so just give up on
 	 * cloning for SDVO encoders.
 	 */
-	intel_sdvo->base.cloneable = false;
+	intel_sdvo->base.cloneable = 0;
 
 	intel_sdvo_select_ddc_bus(dev_priv, intel_sdvo, sdvo_reg);
 
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index b64fc1c..5be4ab2 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1639,9 +1639,8 @@ intel_tv_init(struct drm_device *dev)
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
 	intel_encoder->type = INTEL_OUTPUT_TVOUT;
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
-	intel_encoder->cloneable = false;
+	intel_encoder->cloneable = 0;
 	intel_encoder->base.possible_crtcs = ((1 << 0) | (1 << 1));
-	intel_encoder->base.possible_clones = (1 << INTEL_OUTPUT_TVOUT);
 	intel_tv->type = DRM_MODE_CONNECTOR_Unknown;
 
 	/* BIOS margin values */
-- 
1.8.3.2

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

[PATCH 2/4] drm/i915: Don't use HDMI 12bpc when cloning with other encoder types

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

When cloning HDMI with other output types, we can't use 12bpc since the
clocks for the other encoder types would be off. So have
intel_hdmi_compute_config() check if there are other encoders besides
HDMI being fed from the same pipe, and if so, pick 8bpc insted if 12bpc.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 393cd30..524962d 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -843,6 +843,30 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 	return MODE_OK;
 }
 
+static bool hdmi_12bpc_possible(struct intel_crtc *crtc)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct intel_encoder *encoder;
+	int count = 0, count_hdmi = 0;
+
+	if (!HAS_PCH_SPLIT(dev))
+		return false;
+
+	list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) {
+		if (encoder->new_crtc != crtc)
+			continue;
+
+		count_hdmi += encoder->type == INTEL_OUTPUT_HDMI;
+		count++;
+	}
+
+	/*
+	 * HDMI 12bpc affects the clocks, so it's only possible
+	 * when not cloning with other encoder types.
+	 */
+	return count_hdmi > 0 && count_hdmi == count;
+}
+
 bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 			       struct intel_crtc_config *pipe_config)
 {
@@ -875,7 +899,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	 * within limits.
 	 */
 	if (pipe_config->pipe_bpp > 8*3 && intel_hdmi->has_hdmi_sink &&
-	    clock_12bpc <= portclock_limit && HAS_PCH_SPLIT(dev)) {
+	    clock_12bpc <= portclock_limit &&
+	    hdmi_12bpc_possible(encoder->new_crtc)) {
 		DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
 		desired_bpp = 12*3;
 
-- 
1.8.3.2

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

[PATCH 3/4] drm/i915: Allow HDMI+VGA cloning

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

HDMI+VGA cloning should be supported on all platforms. The only real
obstacle is the 1.5x clock adjustment for 12bpc HDMI, but that is now
taken care of, so we can allow HDMI+VGA cloning.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73850
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c  | 2 +-
 drivers/gpu/drm/i915/intel_hdmi.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index da37930..c0b40ab 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -800,7 +800,7 @@ void intel_crt_init(struct drm_device *dev)
 	intel_connector_attach_encoder(intel_connector, &crt->base);
 
 	crt->base.type = INTEL_OUTPUT_ANALOG;
-	crt->base.cloneable = 1 << INTEL_OUTPUT_DVO;
+	crt->base.cloneable = (1 << INTEL_OUTPUT_DVO) | (1 << INTEL_OUTPUT_HDMI);
 	if (IS_I830(dev))
 		crt->base.crtc_mask = (1 << 0);
 	else
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 524962d..9dacb33 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1315,7 +1315,7 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
 
 	intel_encoder->type = INTEL_OUTPUT_HDMI;
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
-	intel_encoder->cloneable = 0;
+	intel_encoder->cloneable = 1 << INTEL_OUTPUT_ANALOG;
 
 	intel_dig_port->port = port;
 	intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
-- 
1.8.3.2

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

[PATCH 4/4] drm/i915: Allow HDMI+HDMI cloning on g4x

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

BSpec is a bit unclear whether HDMI+HDMI cloning should work on g4x.
Tests on real hardware say that it does. Since g4x can't send
infoframes to more than one HDMI port anyway, we don't lose anything
by allow it.

For PCH platforms BSpec explicitly forbids HDMI+HDMI cloning.

Whether HDMI+HDMI cloning might also work on VLV is a bit unclear, but
since we'd at least lose the capability of sending infoframes to more
than one cloned HDMI port, it doesn't seem like a good idea to allow it.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73850
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 9dacb33..4eb30d8 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1316,6 +1316,13 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
 	intel_encoder->type = INTEL_OUTPUT_HDMI;
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
 	intel_encoder->cloneable = 1 << INTEL_OUTPUT_ANALOG;
+	/*
+	 * BSpec is unclear about HDMI+HDMI cloning on g4x, but it seems
+	 * to work on real hardware. And since g4x can send infoframes to
+	 * only one port anyway, nothing is lost by allowing it.
+	 */
+	if (IS_G4X(dev))
+		intel_encoder->cloneable |= 1 << INTEL_OUTPUT_HDMI;
 
 	intel_dig_port->port = port;
 	intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
-- 
1.8.3.2

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

Re: [PATCH 1/4] drm/i915: Make encoder cloning more flexible

[Daniel Vetter]

On Mon, Mar 03, 2014 at 04:15:28PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we allow encoders to indicate whether they can be part of a
> cloned set with just one flag. That's not flexible enough to describe
> the actual hardware capabilities. Instead make it a bitmask of encoder
> types with which the current encoder can be cloned.
> 
> For now we set the bitmask to allow DVO+DVO and DVO+VGA, which should
> match what the old boolean flag allowed. We will add some more cloning
> options in the future.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Do we really need this? With your patches we now also allow hdmi/vga in a
clone group. hdmi+dvo never shipped in the same system, so there's no
restriction.

At least when I've looked at this when doing the modeset rework docs
indicated that (excluding ridiculous stuff) there's only really one
cloning group on any given platform.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_crt.c     |  2 +-
>  drivers/gpu/drm/i915/intel_ddi.c     |  2 +-
>  drivers/gpu/drm/i915/intel_display.c | 53 ++++++++++++++++++++++++------------
>  drivers/gpu/drm/i915/intel_dp.c      |  2 +-
>  drivers/gpu/drm/i915/intel_drv.h     |  6 +---
>  drivers/gpu/drm/i915/intel_dsi.c     |  2 +-
>  drivers/gpu/drm/i915/intel_dvo.c     |  5 ++--
>  drivers/gpu/drm/i915/intel_hdmi.c    |  2 +-
>  drivers/gpu/drm/i915/intel_lvds.c    |  2 +-
>  drivers/gpu/drm/i915/intel_sdvo.c    |  2 +-
>  drivers/gpu/drm/i915/intel_tv.c      |  3 +-
>  11 files changed, 48 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 9864aa1..da37930 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -800,7 +800,7 @@ void intel_crt_init(struct drm_device *dev)
>  	intel_connector_attach_encoder(intel_connector, &crt->base);
>  
>  	crt->base.type = INTEL_OUTPUT_ANALOG;
> -	crt->base.cloneable = true;
> +	crt->base.cloneable = 1 << INTEL_OUTPUT_DVO;
>  	if (IS_I830(dev))
>  		crt->base.crtc_mask = (1 << 0);
>  	else
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 2643d3b..a6f77b7 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1712,7 +1712,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>  
>  	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
>  	intel_encoder->crtc_mask =  (1 << 0) | (1 << 1) | (1 << 2);
> -	intel_encoder->cloneable = false;
> +	intel_encoder->cloneable = 0;
>  	intel_encoder->hot_plug = intel_ddi_hot_plug;
>  
>  	if (init_dp)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6f15627..677543c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8960,23 +8960,47 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
>  }
>  
> -static bool check_encoder_cloning(struct drm_crtc *crtc)
> +static bool encoders_cloneable(const struct intel_encoder *a,
> +			       const struct intel_encoder *b)
>  {
> -	int num_encoders = 0;
> -	bool uncloneable_encoders = false;
> +	/* masks could be asymmetric, so check both ways */
> +	return a == b || (a->cloneable & (1 << b->type) &&
> +			  b->cloneable & (1 << a->type));
> +}
> +
> +static bool check_single_encoder_cloning(struct intel_crtc *crtc,
> +					 struct intel_encoder *encoder)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct intel_encoder *source_encoder;
> +
> +	list_for_each_entry(source_encoder,
> +			    &dev->mode_config.encoder_list, base.head) {
> +		if (source_encoder->new_crtc != crtc)
> +			continue;
> +
> +		if (!encoders_cloneable(encoder, source_encoder))
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
> +static bool check_encoder_cloning(struct intel_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->base.dev;
>  	struct intel_encoder *encoder;
>  
> -	list_for_each_entry(encoder, &crtc->dev->mode_config.encoder_list,
> -			    base.head) {
> -		if (&encoder->new_crtc->base != crtc)
> +	list_for_each_entry(encoder,
> +			    &dev->mode_config.encoder_list, base.head) {
> +		if (encoder->new_crtc != crtc)
>  			continue;
>  
> -		num_encoders++;
> -		if (!encoder->cloneable)
> -			uncloneable_encoders = true;
> +		if (!check_single_encoder_cloning(crtc, encoder))
> +			return false;
>  	}
>  
> -	return !(num_encoders > 1 && uncloneable_encoders);
> +	return true;
>  }
>  
>  static struct intel_crtc_config *
> @@ -8990,7 +9014,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  	int plane_bpp, ret = -EINVAL;
>  	bool retry = true;
>  
> -	if (!check_encoder_cloning(crtc)) {
> +	if (!check_encoder_cloning(to_intel_crtc(crtc))) {
>  		DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
>  		return ERR_PTR(-EINVAL);
>  	}
> @@ -10353,12 +10377,7 @@ static int intel_encoder_clones(struct intel_encoder *encoder)
>  
>  	list_for_each_entry(source_encoder,
>  			    &dev->mode_config.encoder_list, base.head) {
> -
> -		if (encoder == source_encoder)
> -			index_mask |= (1 << entry);
> -
> -		/* Intel hw has only one MUX where enocoders could be cloned. */
> -		if (encoder->cloneable && source_encoder->cloneable)
> +		if (encoders_cloneable(encoder, source_encoder))
>  			index_mask |= (1 << entry);
>  
>  		entry++;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index c512d78..3bc6aad 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3954,7 +3954,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  
>  	intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> -	intel_encoder->cloneable = false;
> +	intel_encoder->cloneable = 0;
>  	intel_encoder->hot_plug = intel_dp_hot_plug;
>  
>  	if (!intel_dp_init_connector(intel_dig_port, intel_connector)) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a4ffc02..76fd998 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -124,11 +124,7 @@ struct intel_encoder {
>  	struct intel_crtc *new_crtc;
>  
>  	int type;
> -	/*
> -	 * Intel hw has only one MUX where encoders could be clone, hence a
> -	 * simple flag is enough to compute the possible_clones mask.
> -	 */
> -	bool cloneable;
> +	unsigned int cloneable;
>  	bool connectors_active;
>  	void (*hot_plug)(struct intel_encoder *);
>  	bool (*compute_config)(struct intel_encoder *,
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 3ee1db1..36fb7f9 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -604,7 +604,7 @@ bool intel_dsi_init(struct drm_device *dev)
>  	intel_encoder->type = INTEL_OUTPUT_DSI;
>  	intel_encoder->crtc_mask = (1 << 0); /* XXX */
>  
> -	intel_encoder->cloneable = false;
> +	intel_encoder->cloneable = 0;
>  	drm_connector_init(dev, connector, &intel_dsi_connector_funcs,
>  			   DRM_MODE_CONNECTOR_DSI);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 86eeb8b..7fe3fee 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -522,14 +522,15 @@ void intel_dvo_init(struct drm_device *dev)
>  		intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
>  		switch (dvo->type) {
>  		case INTEL_DVO_CHIP_TMDS:
> -			intel_encoder->cloneable = true;
> +			intel_encoder->cloneable = (1 << INTEL_OUTPUT_ANALOG) |
> +				(1 << INTEL_OUTPUT_DVO);
>  			drm_connector_init(dev, connector,
>  					   &intel_dvo_connector_funcs,
>  					   DRM_MODE_CONNECTOR_DVII);
>  			encoder_type = DRM_MODE_ENCODER_TMDS;
>  			break;
>  		case INTEL_DVO_CHIP_LVDS:
> -			intel_encoder->cloneable = false;
> +			intel_encoder->cloneable = 0;
>  			drm_connector_init(dev, connector,
>  					   &intel_dvo_connector_funcs,
>  					   DRM_MODE_CONNECTOR_LVDS);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index aa4641d..393cd30 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1290,7 +1290,7 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
>  
>  	intel_encoder->type = INTEL_OUTPUT_HDMI;
>  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> -	intel_encoder->cloneable = false;
> +	intel_encoder->cloneable = 0;
>  
>  	intel_dig_port->port = port;
>  	intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index fecff3c..ef5e566 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -963,7 +963,7 @@ void intel_lvds_init(struct drm_device *dev)
>  	intel_connector_attach_encoder(intel_connector, intel_encoder);
>  	intel_encoder->type = INTEL_OUTPUT_LVDS;
>  
> -	intel_encoder->cloneable = false;
> +	intel_encoder->cloneable = 0;
>  	if (HAS_PCH_SPLIT(dev))
>  		intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>  	else if (IS_GEN4(dev))
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 825853d..9a0b71f 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -3032,7 +3032,7 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
>  	 * simplistic anyway to express such constraints, so just give up on
>  	 * cloning for SDVO encoders.
>  	 */
> -	intel_sdvo->base.cloneable = false;
> +	intel_sdvo->base.cloneable = 0;
>  
>  	intel_sdvo_select_ddc_bus(dev_priv, intel_sdvo, sdvo_reg);
>  
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index b64fc1c..5be4ab2 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1639,9 +1639,8 @@ intel_tv_init(struct drm_device *dev)
>  	intel_connector_attach_encoder(intel_connector, intel_encoder);
>  	intel_encoder->type = INTEL_OUTPUT_TVOUT;
>  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
> -	intel_encoder->cloneable = false;
> +	intel_encoder->cloneable = 0;
>  	intel_encoder->base.possible_crtcs = ((1 << 0) | (1 << 1));
> -	intel_encoder->base.possible_clones = (1 << INTEL_OUTPUT_TVOUT);
>  	intel_tv->type = DRM_MODE_CONNECTOR_Unknown;
>  
>  	/* BIOS margin values */
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 4/4] drm/i915: Allow HDMI+HDMI cloning on g4x

[Daniel Vetter]

On Mon, Mar 03, 2014 at 04:15:31PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> BSpec is a bit unclear whether HDMI+HDMI cloning should work on g4x.
> Tests on real hardware say that it does. Since g4x can't send
> infoframes to more than one HDMI port anyway, we don't lose anything
> by allow it.
> 
> For PCH platforms BSpec explicitly forbids HDMI+HDMI cloning.
> 
> Whether HDMI+HDMI cloning might also work on VLV is a bit unclear, but
> since we'd at least lose the capability of sending infoframes to more
> than one cloned HDMI port, it doesn't seem like a good idea to allow it.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73850
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 9dacb33..4eb30d8 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1316,6 +1316,13 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
>  	intel_encoder->type = INTEL_OUTPUT_HDMI;
>  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>  	intel_encoder->cloneable = 1 << INTEL_OUTPUT_ANALOG;
> +	/*
> +	 * BSpec is unclear about HDMI+HDMI cloning on g4x, but it seems
> +	 * to work on real hardware. And since g4x can send infoframes to
> +	 * only one port anyway, nothing is lost by allowing it.
> +	 */
> +	if (IS_G4X(dev))

Why restrict this? The per-encoder infoframe setup should dtrt everywhere
else I hope ...
-Daniel

> +		intel_encoder->cloneable |= 1 << INTEL_OUTPUT_HDMI;
>  
>  	intel_dig_port->port = port;
>  	intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 4/4] drm/i915: Allow HDMI+HDMI cloning on g4x

[Ville Syrjälä]

On Wed, Mar 05, 2014 at 07:21:58PM +0100, Daniel Vetter wrote:
> On Mon, Mar 03, 2014 at 04:15:31PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > BSpec is a bit unclear whether HDMI+HDMI cloning should work on g4x.
> > Tests on real hardware say that it does. Since g4x can't send
> > infoframes to more than one HDMI port anyway, we don't lose anything
> > by allow it.
> > 
> > For PCH platforms BSpec explicitly forbids HDMI+HDMI cloning.
> > 
> > Whether HDMI+HDMI cloning might also work on VLV is a bit unclear, but
> > since we'd at least lose the capability of sending infoframes to more
> > than one cloned HDMI port, it doesn't seem like a good idea to allow it.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73850
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_hdmi.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 9dacb33..4eb30d8 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1316,6 +1316,13 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
> >  	intel_encoder->type = INTEL_OUTPUT_HDMI;
> >  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> >  	intel_encoder->cloneable = 1 << INTEL_OUTPUT_ANALOG;
> > +	/*
> > +	 * BSpec is unclear about HDMI+HDMI cloning on g4x, but it seems
> > +	 * to work on real hardware. And since g4x can send infoframes to
> > +	 * only one port anyway, nothing is lost by allowing it.
> > +	 */
> > +	if (IS_G4X(dev))
> 
> Why restrict this? The per-encoder infoframe setup should dtrt everywhere
> else I hope ...

As stated in the commit message, BSpec forbids it for PCH.

Additionally the video DIP lives is in the transcoder, and
one transcoder apparently can't send infoframes to more than
one port. I'm not sure how this stuff actually gets routed
in the hardware, but the video DIP control register ususally
has its own port selector bits which can only select one port
at a time.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 1/4] drm/i915: Make encoder cloning more flexible

[Ville Syrjälä]

On Wed, Mar 05, 2014 at 07:20:51PM +0100, Daniel Vetter wrote:
> On Mon, Mar 03, 2014 at 04:15:28PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Currently we allow encoders to indicate whether they can be part of a
> > cloned set with just one flag. That's not flexible enough to describe
> > the actual hardware capabilities. Instead make it a bitmask of encoder
> > types with which the current encoder can be cloned.
> > 
> > For now we set the bitmask to allow DVO+DVO and DVO+VGA, which should
> > match what the old boolean flag allowed. We will add some more cloning
> > options in the future.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Do we really need this? With your patches we now also allow hdmi/vga in a
> clone group. hdmi+dvo never shipped in the same system, so there's no
> restriction.
> 
> At least when I've looked at this when doing the modeset rework docs
> indicated that (excluding ridiculous stuff) there's only really one
> cloning group on any given platform.

Not sure what you consider ridicilous. But generally HDMI+HDMI isn't
allowed, HDMI+VGA works, as does VGA+LVDS, but HDMI+LVDS doesn't work.
SDVO would also allow all kinds of cloning.

Not that I've enabled LVDS (or SDVO) cloning, but that's just because
of the way i915 interprets the mode from the KMS API. If we would treat
is as the actual mode, and we'd have another way to set up the pfit
source size, we could easily allow a lot more.

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_crt.c     |  2 +-
> >  drivers/gpu/drm/i915/intel_ddi.c     |  2 +-
> >  drivers/gpu/drm/i915/intel_display.c | 53 ++++++++++++++++++++++++------------
> >  drivers/gpu/drm/i915/intel_dp.c      |  2 +-
> >  drivers/gpu/drm/i915/intel_drv.h     |  6 +---
> >  drivers/gpu/drm/i915/intel_dsi.c     |  2 +-
> >  drivers/gpu/drm/i915/intel_dvo.c     |  5 ++--
> >  drivers/gpu/drm/i915/intel_hdmi.c    |  2 +-
> >  drivers/gpu/drm/i915/intel_lvds.c    |  2 +-
> >  drivers/gpu/drm/i915/intel_sdvo.c    |  2 +-
> >  drivers/gpu/drm/i915/intel_tv.c      |  3 +-
> >  11 files changed, 48 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> > index 9864aa1..da37930 100644
> > --- a/drivers/gpu/drm/i915/intel_crt.c
> > +++ b/drivers/gpu/drm/i915/intel_crt.c
> > @@ -800,7 +800,7 @@ void intel_crt_init(struct drm_device *dev)
> >  	intel_connector_attach_encoder(intel_connector, &crt->base);
> >  
> >  	crt->base.type = INTEL_OUTPUT_ANALOG;
> > -	crt->base.cloneable = true;
> > +	crt->base.cloneable = 1 << INTEL_OUTPUT_DVO;
> >  	if (IS_I830(dev))
> >  		crt->base.crtc_mask = (1 << 0);
> >  	else
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 2643d3b..a6f77b7 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1712,7 +1712,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> >  
> >  	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
> >  	intel_encoder->crtc_mask =  (1 << 0) | (1 << 1) | (1 << 2);
> > -	intel_encoder->cloneable = false;
> > +	intel_encoder->cloneable = 0;
> >  	intel_encoder->hot_plug = intel_ddi_hot_plug;
> >  
> >  	if (init_dp)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 6f15627..677543c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8960,23 +8960,47 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
> >  	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
> >  }
> >  
> > -static bool check_encoder_cloning(struct drm_crtc *crtc)
> > +static bool encoders_cloneable(const struct intel_encoder *a,
> > +			       const struct intel_encoder *b)
> >  {
> > -	int num_encoders = 0;
> > -	bool uncloneable_encoders = false;
> > +	/* masks could be asymmetric, so check both ways */
> > +	return a == b || (a->cloneable & (1 << b->type) &&
> > +			  b->cloneable & (1 << a->type));
> > +}
> > +
> > +static bool check_single_encoder_cloning(struct intel_crtc *crtc,
> > +					 struct intel_encoder *encoder)
> > +{
> > +	struct drm_device *dev = crtc->base.dev;
> > +	struct intel_encoder *source_encoder;
> > +
> > +	list_for_each_entry(source_encoder,
> > +			    &dev->mode_config.encoder_list, base.head) {
> > +		if (source_encoder->new_crtc != crtc)
> > +			continue;
> > +
> > +		if (!encoders_cloneable(encoder, source_encoder))
> > +			return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +static bool check_encoder_cloning(struct intel_crtc *crtc)
> > +{
> > +	struct drm_device *dev = crtc->base.dev;
> >  	struct intel_encoder *encoder;
> >  
> > -	list_for_each_entry(encoder, &crtc->dev->mode_config.encoder_list,
> > -			    base.head) {
> > -		if (&encoder->new_crtc->base != crtc)
> > +	list_for_each_entry(encoder,
> > +			    &dev->mode_config.encoder_list, base.head) {
> > +		if (encoder->new_crtc != crtc)
> >  			continue;
> >  
> > -		num_encoders++;
> > -		if (!encoder->cloneable)
> > -			uncloneable_encoders = true;
> > +		if (!check_single_encoder_cloning(crtc, encoder))
> > +			return false;
> >  	}
> >  
> > -	return !(num_encoders > 1 && uncloneable_encoders);
> > +	return true;
> >  }
> >  
> >  static struct intel_crtc_config *
> > @@ -8990,7 +9014,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> >  	int plane_bpp, ret = -EINVAL;
> >  	bool retry = true;
> >  
> > -	if (!check_encoder_cloning(crtc)) {
> > +	if (!check_encoder_cloning(to_intel_crtc(crtc))) {
> >  		DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
> >  		return ERR_PTR(-EINVAL);
> >  	}
> > @@ -10353,12 +10377,7 @@ static int intel_encoder_clones(struct intel_encoder *encoder)
> >  
> >  	list_for_each_entry(source_encoder,
> >  			    &dev->mode_config.encoder_list, base.head) {
> > -
> > -		if (encoder == source_encoder)
> > -			index_mask |= (1 << entry);
> > -
> > -		/* Intel hw has only one MUX where enocoders could be cloned. */
> > -		if (encoder->cloneable && source_encoder->cloneable)
> > +		if (encoders_cloneable(encoder, source_encoder))
> >  			index_mask |= (1 << entry);
> >  
> >  		entry++;
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index c512d78..3bc6aad 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3954,7 +3954,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> >  
> >  	intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> >  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> > -	intel_encoder->cloneable = false;
> > +	intel_encoder->cloneable = 0;
> >  	intel_encoder->hot_plug = intel_dp_hot_plug;
> >  
> >  	if (!intel_dp_init_connector(intel_dig_port, intel_connector)) {
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index a4ffc02..76fd998 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -124,11 +124,7 @@ struct intel_encoder {
> >  	struct intel_crtc *new_crtc;
> >  
> >  	int type;
> > -	/*
> > -	 * Intel hw has only one MUX where encoders could be clone, hence a
> > -	 * simple flag is enough to compute the possible_clones mask.
> > -	 */
> > -	bool cloneable;
> > +	unsigned int cloneable;
> >  	bool connectors_active;
> >  	void (*hot_plug)(struct intel_encoder *);
> >  	bool (*compute_config)(struct intel_encoder *,
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index 3ee1db1..36fb7f9 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -604,7 +604,7 @@ bool intel_dsi_init(struct drm_device *dev)
> >  	intel_encoder->type = INTEL_OUTPUT_DSI;
> >  	intel_encoder->crtc_mask = (1 << 0); /* XXX */
> >  
> > -	intel_encoder->cloneable = false;
> > +	intel_encoder->cloneable = 0;
> >  	drm_connector_init(dev, connector, &intel_dsi_connector_funcs,
> >  			   DRM_MODE_CONNECTOR_DSI);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> > index 86eeb8b..7fe3fee 100644
> > --- a/drivers/gpu/drm/i915/intel_dvo.c
> > +++ b/drivers/gpu/drm/i915/intel_dvo.c
> > @@ -522,14 +522,15 @@ void intel_dvo_init(struct drm_device *dev)
> >  		intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
> >  		switch (dvo->type) {
> >  		case INTEL_DVO_CHIP_TMDS:
> > -			intel_encoder->cloneable = true;
> > +			intel_encoder->cloneable = (1 << INTEL_OUTPUT_ANALOG) |
> > +				(1 << INTEL_OUTPUT_DVO);
> >  			drm_connector_init(dev, connector,
> >  					   &intel_dvo_connector_funcs,
> >  					   DRM_MODE_CONNECTOR_DVII);
> >  			encoder_type = DRM_MODE_ENCODER_TMDS;
> >  			break;
> >  		case INTEL_DVO_CHIP_LVDS:
> > -			intel_encoder->cloneable = false;
> > +			intel_encoder->cloneable = 0;
> >  			drm_connector_init(dev, connector,
> >  					   &intel_dvo_connector_funcs,
> >  					   DRM_MODE_CONNECTOR_LVDS);
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index aa4641d..393cd30 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1290,7 +1290,7 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
> >  
> >  	intel_encoder->type = INTEL_OUTPUT_HDMI;
> >  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> > -	intel_encoder->cloneable = false;
> > +	intel_encoder->cloneable = 0;
> >  
> >  	intel_dig_port->port = port;
> >  	intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
> > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > index fecff3c..ef5e566 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -963,7 +963,7 @@ void intel_lvds_init(struct drm_device *dev)
> >  	intel_connector_attach_encoder(intel_connector, intel_encoder);
> >  	intel_encoder->type = INTEL_OUTPUT_LVDS;
> >  
> > -	intel_encoder->cloneable = false;
> > +	intel_encoder->cloneable = 0;
> >  	if (HAS_PCH_SPLIT(dev))
> >  		intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> >  	else if (IS_GEN4(dev))
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> > index 825853d..9a0b71f 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -3032,7 +3032,7 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
> >  	 * simplistic anyway to express such constraints, so just give up on
> >  	 * cloning for SDVO encoders.
> >  	 */
> > -	intel_sdvo->base.cloneable = false;
> > +	intel_sdvo->base.cloneable = 0;
> >  
> >  	intel_sdvo_select_ddc_bus(dev_priv, intel_sdvo, sdvo_reg);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> > index b64fc1c..5be4ab2 100644
> > --- a/drivers/gpu/drm/i915/intel_tv.c
> > +++ b/drivers/gpu/drm/i915/intel_tv.c
> > @@ -1639,9 +1639,8 @@ intel_tv_init(struct drm_device *dev)
> >  	intel_connector_attach_encoder(intel_connector, intel_encoder);
> >  	intel_encoder->type = INTEL_OUTPUT_TVOUT;
> >  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
> > -	intel_encoder->cloneable = false;
> > +	intel_encoder->cloneable = 0;
> >  	intel_encoder->base.possible_crtcs = ((1 << 0) | (1 << 1));
> > -	intel_encoder->base.possible_clones = (1 << INTEL_OUTPUT_TVOUT);
> >  	intel_tv->type = DRM_MODE_CONNECTOR_Unknown;
> >  
> >  	/* BIOS margin values */
> > -- 
> > 1.8.3.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 1/4] drm/i915: Make encoder cloning more flexible

[Rodrigo Vivi]

On Mon, Mar 3, 2014 at 11:15 AM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently we allow encoders to indicate whether they can be part of a
> cloned set with just one flag. That's not flexible enough to describe
> the actual hardware capabilities. Instead make it a bitmask of encoder
> types with which the current encoder can be cloned.
>
> For now we set the bitmask to allow DVO+DVO and DVO+VGA, which should
> match what the old boolean flag allowed. We will add some more cloning
> options in the future.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_crt.c     |  2 +-
>  drivers/gpu/drm/i915/intel_ddi.c     |  2 +-
>  drivers/gpu/drm/i915/intel_display.c | 53 ++++++++++++++++++++++++------------
>  drivers/gpu/drm/i915/intel_dp.c      |  2 +-
>  drivers/gpu/drm/i915/intel_drv.h     |  6 +---
>  drivers/gpu/drm/i915/intel_dsi.c     |  2 +-
>  drivers/gpu/drm/i915/intel_dvo.c     |  5 ++--
>  drivers/gpu/drm/i915/intel_hdmi.c    |  2 +-
>  drivers/gpu/drm/i915/intel_lvds.c    |  2 +-
>  drivers/gpu/drm/i915/intel_sdvo.c    |  2 +-
>  drivers/gpu/drm/i915/intel_tv.c      |  3 +-
>  11 files changed, 48 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 9864aa1..da37930 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -800,7 +800,7 @@ void intel_crt_init(struct drm_device *dev)
>         intel_connector_attach_encoder(intel_connector, &crt->base);
>
>         crt->base.type = INTEL_OUTPUT_ANALOG;
> -       crt->base.cloneable = true;
> +       crt->base.cloneable = 1 << INTEL_OUTPUT_DVO;
>         if (IS_I830(dev))
>                 crt->base.crtc_mask = (1 << 0);
>         else
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 2643d3b..a6f77b7 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1712,7 +1712,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>
>         intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
>         intel_encoder->crtc_mask =  (1 << 0) | (1 << 1) | (1 << 2);
> -       intel_encoder->cloneable = false;
> +       intel_encoder->cloneable = 0;
>         intel_encoder->hot_plug = intel_ddi_hot_plug;
>
>         if (init_dp)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6f15627..677543c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8960,23 +8960,47 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>         DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
>  }
>
> -static bool check_encoder_cloning(struct drm_crtc *crtc)
> +static bool encoders_cloneable(const struct intel_encoder *a,
> +                              const struct intel_encoder *b)
>  {
> -       int num_encoders = 0;
> -       bool uncloneable_encoders = false;
> +       /* masks could be asymmetric, so check both ways */
> +       return a == b || (a->cloneable & (1 << b->type) &&
> +                         b->cloneable & (1 << a->type));
> +}
> +
> +static bool check_single_encoder_cloning(struct intel_crtc *crtc,
> +                                        struct intel_encoder *encoder)
> +{
> +       struct drm_device *dev = crtc->base.dev;
> +       struct intel_encoder *source_encoder;
> +
> +       list_for_each_entry(source_encoder,
> +                           &dev->mode_config.encoder_list, base.head) {
> +               if (source_encoder->new_crtc != crtc)
> +                       continue;
> +
> +               if (!encoders_cloneable(encoder, source_encoder))
> +                       return false;
> +       }
> +
> +       return true;
> +}
> +
> +static bool check_encoder_cloning(struct intel_crtc *crtc)
> +{
> +       struct drm_device *dev = crtc->base.dev;
>         struct intel_encoder *encoder;
>
> -       list_for_each_entry(encoder, &crtc->dev->mode_config.encoder_list,
> -                           base.head) {
> -               if (&encoder->new_crtc->base != crtc)
> +       list_for_each_entry(encoder,
> +                           &dev->mode_config.encoder_list, base.head) {
> +               if (encoder->new_crtc != crtc)
>                         continue;
>
> -               num_encoders++;
> -               if (!encoder->cloneable)
> -                       uncloneable_encoders = true;
> +               if (!check_single_encoder_cloning(crtc, encoder))
> +                       return false;
>         }
>
> -       return !(num_encoders > 1 && uncloneable_encoders);
> +       return true;
>  }
>
>  static struct intel_crtc_config *
> @@ -8990,7 +9014,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>         int plane_bpp, ret = -EINVAL;
>         bool retry = true;
>
> -       if (!check_encoder_cloning(crtc)) {
> +       if (!check_encoder_cloning(to_intel_crtc(crtc))) {
>                 DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
>                 return ERR_PTR(-EINVAL);
>         }
> @@ -10353,12 +10377,7 @@ static int intel_encoder_clones(struct intel_encoder *encoder)
>
>         list_for_each_entry(source_encoder,
>                             &dev->mode_config.encoder_list, base.head) {
> -
> -               if (encoder == source_encoder)
> -                       index_mask |= (1 << entry);
> -
> -               /* Intel hw has only one MUX where enocoders could be cloned. */
> -               if (encoder->cloneable && source_encoder->cloneable)
> +               if (encoders_cloneable(encoder, source_encoder))
>                         index_mask |= (1 << entry);
>
>                 entry++;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index c512d78..3bc6aad 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3954,7 +3954,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>
>         intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>         intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> -       intel_encoder->cloneable = false;
> +       intel_encoder->cloneable = 0;
>         intel_encoder->hot_plug = intel_dp_hot_plug;
>
>         if (!intel_dp_init_connector(intel_dig_port, intel_connector)) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a4ffc02..76fd998 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -124,11 +124,7 @@ struct intel_encoder {
>         struct intel_crtc *new_crtc;
>
>         int type;
> -       /*
> -        * Intel hw has only one MUX where encoders could be clone, hence a
> -        * simple flag is enough to compute the possible_clones mask.
> -        */
> -       bool cloneable;
> +       unsigned int cloneable;
>         bool connectors_active;
>         void (*hot_plug)(struct intel_encoder *);
>         bool (*compute_config)(struct intel_encoder *,
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 3ee1db1..36fb7f9 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -604,7 +604,7 @@ bool intel_dsi_init(struct drm_device *dev)
>         intel_encoder->type = INTEL_OUTPUT_DSI;
>         intel_encoder->crtc_mask = (1 << 0); /* XXX */
>
> -       intel_encoder->cloneable = false;
> +       intel_encoder->cloneable = 0;
>         drm_connector_init(dev, connector, &intel_dsi_connector_funcs,
>                            DRM_MODE_CONNECTOR_DSI);
>
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 86eeb8b..7fe3fee 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -522,14 +522,15 @@ void intel_dvo_init(struct drm_device *dev)
>                 intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
>                 switch (dvo->type) {
>                 case INTEL_DVO_CHIP_TMDS:
> -                       intel_encoder->cloneable = true;
> +                       intel_encoder->cloneable = (1 << INTEL_OUTPUT_ANALOG) |
> +                               (1 << INTEL_OUTPUT_DVO);
>                         drm_connector_init(dev, connector,
>                                            &intel_dvo_connector_funcs,
>                                            DRM_MODE_CONNECTOR_DVII);
>                         encoder_type = DRM_MODE_ENCODER_TMDS;
>                         break;
>                 case INTEL_DVO_CHIP_LVDS:
> -                       intel_encoder->cloneable = false;
> +                       intel_encoder->cloneable = 0;
>                         drm_connector_init(dev, connector,
>                                            &intel_dvo_connector_funcs,
>                                            DRM_MODE_CONNECTOR_LVDS);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index aa4641d..393cd30 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1290,7 +1290,7 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
>
>         intel_encoder->type = INTEL_OUTPUT_HDMI;
>         intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> -       intel_encoder->cloneable = false;
> +       intel_encoder->cloneable = 0;
>
>         intel_dig_port->port = port;
>         intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index fecff3c..ef5e566 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -963,7 +963,7 @@ void intel_lvds_init(struct drm_device *dev)
>         intel_connector_attach_encoder(intel_connector, intel_encoder);
>         intel_encoder->type = INTEL_OUTPUT_LVDS;
>
> -       intel_encoder->cloneable = false;
> +       intel_encoder->cloneable = 0;
>         if (HAS_PCH_SPLIT(dev))
>                 intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>         else if (IS_GEN4(dev))
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 825853d..9a0b71f 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -3032,7 +3032,7 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
>          * simplistic anyway to express such constraints, so just give up on
>          * cloning for SDVO encoders.
>          */
> -       intel_sdvo->base.cloneable = false;
> +       intel_sdvo->base.cloneable = 0;
>
>         intel_sdvo_select_ddc_bus(dev_priv, intel_sdvo, sdvo_reg);
>
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index b64fc1c..5be4ab2 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1639,9 +1639,8 @@ intel_tv_init(struct drm_device *dev)
>         intel_connector_attach_encoder(intel_connector, intel_encoder);
>         intel_encoder->type = INTEL_OUTPUT_TVOUT;
>         intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
> -       intel_encoder->cloneable = false;
> +       intel_encoder->cloneable = 0;
>         intel_encoder->base.possible_crtcs = ((1 << 0) | (1 << 1));
> -       intel_encoder->base.possible_clones = (1 << INTEL_OUTPUT_TVOUT);

why did you remove this?
or shouldn't it be a separated patch at least?

The rest of the approach looks ok for me, so with this explained or
fixed feel free to use
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

>         intel_tv->type = DRM_MODE_CONNECTOR_Unknown;
>
>         /* BIOS margin values */
> --
> 1.8.3.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

Re: [PATCH 2/4] drm/i915: Don't use HDMI 12bpc when cloning with other encoder types

[Rodrigo Vivi]

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

On Mon, Mar 3, 2014 at 11:15 AM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> When cloning HDMI with other output types, we can't use 12bpc since the
> clocks for the other encoder types would be off. So have
> intel_hdmi_compute_config() check if there are other encoders besides
> HDMI being fed from the same pipe, and if so, pick 8bpc insted if 12bpc.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 393cd30..524962d 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -843,6 +843,30 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>         return MODE_OK;
>  }
>
> +static bool hdmi_12bpc_possible(struct intel_crtc *crtc)
> +{
> +       struct drm_device *dev = crtc->base.dev;
> +       struct intel_encoder *encoder;
> +       int count = 0, count_hdmi = 0;
> +
> +       if (!HAS_PCH_SPLIT(dev))
> +               return false;
> +
> +       list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) {
> +               if (encoder->new_crtc != crtc)
> +                       continue;
> +
> +               count_hdmi += encoder->type == INTEL_OUTPUT_HDMI;
> +               count++;
> +       }
> +
> +       /*
> +        * HDMI 12bpc affects the clocks, so it's only possible
> +        * when not cloning with other encoder types.
> +        */
> +       return count_hdmi > 0 && count_hdmi == count;
> +}
> +
>  bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>                                struct intel_crtc_config *pipe_config)
>  {
> @@ -875,7 +899,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>          * within limits.
>          */
>         if (pipe_config->pipe_bpp > 8*3 && intel_hdmi->has_hdmi_sink &&
> -           clock_12bpc <= portclock_limit && HAS_PCH_SPLIT(dev)) {
> +           clock_12bpc <= portclock_limit &&
> +           hdmi_12bpc_possible(encoder->new_crtc)) {
>                 DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
>                 desired_bpp = 12*3;
>
> --
> 1.8.3.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

Re: [PATCH 3/4] drm/i915: Allow HDMI+VGA cloning

[Rodrigo Vivi]

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

On Mon, Mar 3, 2014 at 11:15 AM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> HDMI+VGA cloning should be supported on all platforms. The only real
> obstacle is the 1.5x clock adjustment for 12bpc HDMI, but that is now
> taken care of, so we can allow HDMI+VGA cloning.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73850
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_crt.c  | 2 +-
>  drivers/gpu/drm/i915/intel_hdmi.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index da37930..c0b40ab 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -800,7 +800,7 @@ void intel_crt_init(struct drm_device *dev)
>         intel_connector_attach_encoder(intel_connector, &crt->base);
>
>         crt->base.type = INTEL_OUTPUT_ANALOG;
> -       crt->base.cloneable = 1 << INTEL_OUTPUT_DVO;
> +       crt->base.cloneable = (1 << INTEL_OUTPUT_DVO) | (1 << INTEL_OUTPUT_HDMI);
>         if (IS_I830(dev))
>                 crt->base.crtc_mask = (1 << 0);
>         else
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 524962d..9dacb33 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1315,7 +1315,7 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
>
>         intel_encoder->type = INTEL_OUTPUT_HDMI;
>         intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> -       intel_encoder->cloneable = 0;
> +       intel_encoder->cloneable = 1 << INTEL_OUTPUT_ANALOG;
>
>         intel_dig_port->port = port;
>         intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
> --
> 1.8.3.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

Re: [PATCH 4/4] drm/i915: Allow HDMI+HDMI cloning on g4x

[Rodrigo Vivi]

On Mon, Mar 3, 2014 at 11:15 AM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> BSpec is a bit unclear whether HDMI+HDMI cloning should work on g4x.
> Tests on real hardware say that it does. Since g4x can't send
> infoframes to more than one HDMI port anyway, we don't lose anything
> by allow it.

I hope this isn't a case that will work for some and blow up in others.
Anyway i'm in favor of this patch! +1

>
> For PCH platforms BSpec explicitly forbids HDMI+HDMI cloning.

To be honest I couldn't find this info, but sounds resonable and patch
is on the safe side for platforms with PCH.

So, feel free to use
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

>
> Whether HDMI+HDMI cloning might also work on VLV is a bit unclear, but
> since we'd at least lose the capability of sending infoframes to more
> than one cloned HDMI port, it doesn't seem like a good idea to allow it.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73850
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 9dacb33..4eb30d8 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1316,6 +1316,13 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
>         intel_encoder->type = INTEL_OUTPUT_HDMI;
>         intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>         intel_encoder->cloneable = 1 << INTEL_OUTPUT_ANALOG;
> +       /*
> +        * BSpec is unclear about HDMI+HDMI cloning on g4x, but it seems
> +        * to work on real hardware. And since g4x can send infoframes to
> +        * only one port anyway, nothing is lost by allowing it.
> +        */
> +       if (IS_G4X(dev))
> +               intel_encoder->cloneable |= 1 << INTEL_OUTPUT_HDMI;
>
>         intel_dig_port->port = port;
>         intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
> --
> 1.8.3.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

Re: [PATCH 1/4] drm/i915: Make encoder cloning more flexible

[Ville Syrjälä]

On Mon, Mar 10, 2014 at 03:27:09PM -0300, Rodrigo Vivi wrote:
> On Mon, Mar 3, 2014 at 11:15 AM,  <ville.syrjala@linux.intel.com> wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> > index b64fc1c..5be4ab2 100644
> > --- a/drivers/gpu/drm/i915/intel_tv.c
> > +++ b/drivers/gpu/drm/i915/intel_tv.c
> > @@ -1639,9 +1639,8 @@ intel_tv_init(struct drm_device *dev)
> >         intel_connector_attach_encoder(intel_connector, intel_encoder);
> >         intel_encoder->type = INTEL_OUTPUT_TVOUT;
> >         intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
> > -       intel_encoder->cloneable = false;
> > +       intel_encoder->cloneable = 0;
> >         intel_encoder->base.possible_crtcs = ((1 << 0) | (1 << 1));
> > -       intel_encoder->base.possible_clones = (1 << INTEL_OUTPUT_TVOUT);
> 
> why did you remove this?
> or shouldn't it be a separated patch at least?

It has no business being here. We compute possible_clones dynamically
so this will get overwritten. I forgot to mention it in the commits
msg. It could be a separate patch, but since it does nothing just
killing it as part of this patch seems fine.

> 
> The rest of the approach looks ok for me, so with this explained or
> fixed feel free to use
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> 
> >         intel_tv->type = DRM_MODE_CONNECTOR_Unknown;
> >
> >         /* BIOS margin values */
> > --
> > 1.8.3.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 4/4] drm/i915: Allow HDMI+HDMI cloning on g4x

[Daniel Vetter]

On Mon, Mar 10, 2014 at 03:50:40PM -0300, Rodrigo Vivi wrote:
> On Mon, Mar 3, 2014 at 11:15 AM,  <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > BSpec is a bit unclear whether HDMI+HDMI cloning should work on g4x.
> > Tests on real hardware say that it does. Since g4x can't send
> > infoframes to more than one HDMI port anyway, we don't lose anything
> > by allow it.
> 
> I hope this isn't a case that will work for some and blow up in others.
> Anyway i'm in favor of this patch! +1
> 
> >
> > For PCH platforms BSpec explicitly forbids HDMI+HDMI cloning.
> 
> To be honest I couldn't find this info, but sounds resonable and patch
> is on the safe side for platforms with PCH.
> 
> So, feel free to use
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

All four patches merged (with the first patch's commit message pimped to
clarify Rodrigo's question).

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch