[Intel-gfx] [PATCH 1/4] drm/i915: Make encoder cloning more flexible
Ville Syrjälä
ville.syrjala at linux.intel.com
Wed Mar 5 19:47:17 CET 2014
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 at linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala at 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 at 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 at 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
More information about the Intel-gfx
mailing list