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

Rodrigo Vivi rodrigo.vivi at gmail.com
Mon Mar 10 19:27:09 CET 2014


On Mon, Mar 3, 2014 at 11:15 AM,  <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>
> ---
>  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 at gmail.com>

>         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



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



More information about the Intel-gfx mailing list