* [PATCH 0/4] drm/915: Relax output cloning restrictions
@ 2014-03-03 14:15 ville.syrjala
2014-03-03 14:15 ` [PATCH 1/4] drm/i915: Make encoder cloning more flexible ville.syrjala
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: ville.syrjala @ 2014-03-03 14:15 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/4] drm/i915: Make encoder cloning more flexible 2014-03-03 14:15 [PATCH 0/4] drm/915: Relax output cloning restrictions ville.syrjala @ 2014-03-03 14:15 ` ville.syrjala 2014-03-05 18:20 ` Daniel Vetter 2014-03-10 18:27 ` Rodrigo Vivi 2014-03-03 14:15 ` [PATCH 2/4] drm/i915: Don't use HDMI 12bpc when cloning with other encoder types ville.syrjala ` (2 subsequent siblings) 3 siblings, 2 replies; 15+ messages in thread From: ville.syrjala @ 2014-03-03 14:15 UTC (permalink / raw) To: intel-gfx 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] drm/i915: Make encoder cloning more flexible 2014-03-03 14:15 ` [PATCH 1/4] drm/i915: Make encoder cloning more flexible ville.syrjala @ 2014-03-05 18:20 ` Daniel Vetter 2014-03-05 18:47 ` Ville Syrjälä 2014-03-10 18:27 ` Rodrigo Vivi 1 sibling, 1 reply; 15+ messages in thread From: Daniel Vetter @ 2014-03-05 18:20 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] drm/i915: Make encoder cloning more flexible 2014-03-05 18:20 ` Daniel Vetter @ 2014-03-05 18:47 ` Ville Syrjälä 0 siblings, 0 replies; 15+ messages in thread From: Ville Syrjälä @ 2014-03-05 18:47 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] drm/i915: Make encoder cloning more flexible 2014-03-03 14:15 ` [PATCH 1/4] drm/i915: Make encoder cloning more flexible ville.syrjala 2014-03-05 18:20 ` Daniel Vetter @ 2014-03-10 18:27 ` Rodrigo Vivi 2014-03-10 19:17 ` Ville Syrjälä 1 sibling, 1 reply; 15+ messages in thread From: Rodrigo Vivi @ 2014-03-10 18:27 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] drm/i915: Make encoder cloning more flexible 2014-03-10 18:27 ` Rodrigo Vivi @ 2014-03-10 19:17 ` Ville Syrjälä 0 siblings, 0 replies; 15+ messages in thread From: Ville Syrjälä @ 2014-03-10 19:17 UTC (permalink / raw) To: Rodrigo Vivi; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/4] drm/i915: Don't use HDMI 12bpc when cloning with other encoder types 2014-03-03 14:15 [PATCH 0/4] drm/915: Relax output cloning restrictions ville.syrjala 2014-03-03 14:15 ` [PATCH 1/4] drm/i915: Make encoder cloning more flexible ville.syrjala @ 2014-03-03 14:15 ` ville.syrjala 2014-03-10 18:31 ` Rodrigo Vivi 2014-03-03 14:15 ` [PATCH 3/4] drm/i915: Allow HDMI+VGA cloning ville.syrjala 2014-03-03 14:15 ` [PATCH 4/4] drm/i915: Allow HDMI+HDMI cloning on g4x ville.syrjala 3 siblings, 1 reply; 15+ messages in thread From: ville.syrjala @ 2014-03-03 14:15 UTC (permalink / raw) To: intel-gfx 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] drm/i915: Don't use HDMI 12bpc when cloning with other encoder types 2014-03-03 14:15 ` [PATCH 2/4] drm/i915: Don't use HDMI 12bpc when cloning with other encoder types ville.syrjala @ 2014-03-10 18:31 ` Rodrigo Vivi 0 siblings, 0 replies; 15+ messages in thread From: Rodrigo Vivi @ 2014-03-10 18:31 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/4] drm/i915: Allow HDMI+VGA cloning 2014-03-03 14:15 [PATCH 0/4] drm/915: Relax output cloning restrictions ville.syrjala 2014-03-03 14:15 ` [PATCH 1/4] drm/i915: Make encoder cloning more flexible ville.syrjala 2014-03-03 14:15 ` [PATCH 2/4] drm/i915: Don't use HDMI 12bpc when cloning with other encoder types ville.syrjala @ 2014-03-03 14:15 ` ville.syrjala 2014-03-10 18:41 ` Rodrigo Vivi 2014-03-03 14:15 ` [PATCH 4/4] drm/i915: Allow HDMI+HDMI cloning on g4x ville.syrjala 3 siblings, 1 reply; 15+ messages in thread From: ville.syrjala @ 2014-03-03 14:15 UTC (permalink / raw) To: intel-gfx 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] drm/i915: Allow HDMI+VGA cloning 2014-03-03 14:15 ` [PATCH 3/4] drm/i915: Allow HDMI+VGA cloning ville.syrjala @ 2014-03-10 18:41 ` Rodrigo Vivi 0 siblings, 0 replies; 15+ messages in thread From: Rodrigo Vivi @ 2014-03-10 18:41 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/4] drm/i915: Allow HDMI+HDMI cloning on g4x 2014-03-03 14:15 [PATCH 0/4] drm/915: Relax output cloning restrictions ville.syrjala ` (2 preceding siblings ...) 2014-03-03 14:15 ` [PATCH 3/4] drm/i915: Allow HDMI+VGA cloning ville.syrjala @ 2014-03-03 14:15 ` ville.syrjala 2014-03-05 18:21 ` Daniel Vetter 2014-03-10 18:50 ` Rodrigo Vivi 3 siblings, 2 replies; 15+ messages in thread From: ville.syrjala @ 2014-03-03 14:15 UTC (permalink / raw) To: intel-gfx 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] drm/i915: Allow HDMI+HDMI cloning on g4x 2014-03-03 14:15 ` [PATCH 4/4] drm/i915: Allow HDMI+HDMI cloning on g4x ville.syrjala @ 2014-03-05 18:21 ` Daniel Vetter 2014-03-05 18:41 ` Ville Syrjälä 2014-03-10 18:50 ` Rodrigo Vivi 1 sibling, 1 reply; 15+ messages in thread From: Daniel Vetter @ 2014-03-05 18:21 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] drm/i915: Allow HDMI+HDMI cloning on g4x 2014-03-05 18:21 ` Daniel Vetter @ 2014-03-05 18:41 ` Ville Syrjälä 0 siblings, 0 replies; 15+ messages in thread From: Ville Syrjälä @ 2014-03-05 18:41 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] drm/i915: Allow HDMI+HDMI cloning on g4x 2014-03-03 14:15 ` [PATCH 4/4] drm/i915: Allow HDMI+HDMI cloning on g4x ville.syrjala 2014-03-05 18:21 ` Daniel Vetter @ 2014-03-10 18:50 ` Rodrigo Vivi 2014-03-10 20:49 ` Daniel Vetter 1 sibling, 1 reply; 15+ messages in thread From: Rodrigo Vivi @ 2014-03-10 18:50 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] drm/i915: Allow HDMI+HDMI cloning on g4x 2014-03-10 18:50 ` Rodrigo Vivi @ 2014-03-10 20:49 ` Daniel Vetter 0 siblings, 0 replies; 15+ messages in thread From: Daniel Vetter @ 2014-03-10 20:49 UTC (permalink / raw) To: Rodrigo Vivi; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2014-03-10 20:49 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-03 14:15 [PATCH 0/4] drm/915: Relax output cloning restrictions ville.syrjala 2014-03-03 14:15 ` [PATCH 1/4] drm/i915: Make encoder cloning more flexible ville.syrjala 2014-03-05 18:20 ` Daniel Vetter 2014-03-05 18:47 ` Ville Syrjälä 2014-03-10 18:27 ` Rodrigo Vivi 2014-03-10 19:17 ` Ville Syrjälä 2014-03-03 14:15 ` [PATCH 2/4] drm/i915: Don't use HDMI 12bpc when cloning with other encoder types ville.syrjala 2014-03-10 18:31 ` Rodrigo Vivi 2014-03-03 14:15 ` [PATCH 3/4] drm/i915: Allow HDMI+VGA cloning ville.syrjala 2014-03-10 18:41 ` Rodrigo Vivi 2014-03-03 14:15 ` [PATCH 4/4] drm/i915: Allow HDMI+HDMI cloning on g4x ville.syrjala 2014-03-05 18:21 ` Daniel Vetter 2014-03-05 18:41 ` Ville Syrjälä 2014-03-10 18:50 ` Rodrigo Vivi 2014-03-10 20:49 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox