[Intel-gfx] [PATCH] drm/i915: simplify possible_clones computation
Paulo Zanoni
przanoni at gmail.com
Thu Jul 12 21:48:13 CEST 2012
2012/7/12 Daniel Vetter <daniel.vetter at ffwll.ch>:
> Intel hw only has one MUX for encoders, so outputs are either not
> cloneable or all in the same group of cloneable outputs. This neatly
> simplifies the code and allows us to ditch some ugly if cascades in
> the dp and hdmi init code (well, we need these if cascades for other
> stuff still, but that can be taken care of in follow-up patches).
>
> Note that this changes two things:
> - dvo can now be cloned with sdvo, but dvo is gen2 whereas sdvo is
> gen3+, so no problem. Note that the old code had a bug and didn't
> allow cloning crt with dvo (but only the other way round).
> - sdvo-lvds can now be cloned with sdvo-non-tv. Spec says this won't
> work, but the only reason I've found is that you can't use the
> panel-fitter (used for lvds upscaling) with anything else. But we
> don't use the panel fitter for sdvo-lvds. Imo this part of Bspec is
> a) rather confusing b) mostly as a guideline to implementors (i.e.
> explicitly stating what is already implicit from the spec, without
> always going into the details of why). So I think we can ignore this
> - worst case we'll get a bug report from a user with with sdvo-lvds
> and sdvo-tmds and have to add that special case back in.
>
> Because sdvo lvds is a bit special explain in comments why sdvo LVDS
> outputs can be cloned, but native LVDS and eDP can't be cloned - we
> use the panel fitter for the later, but not for sdvo.
>
> Note that this also uncoditionally initializes the panel_vdd work used
> by eDP. Trying to be clever doesn't buy us anything (but strange bugs)
> and this way we can kill the is_edp check.
>
> v2: Incorporate review from Paulo
> - Add in a missing space.
> - Pimp comment message to address his concerns.
>
> Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
If you try to commit this to dinq you'll get a conflict on
intel_drv.h. This is easy to fix, so you can keep the R-B after that
:)
> ---
> drivers/gpu/drm/i915/intel_crt.c | 4 +---
> drivers/gpu/drm/i915/intel_display.c | 18 +++++++++++++-----
> drivers/gpu/drm/i915/intel_dp.c | 14 +++-----------
> drivers/gpu/drm/i915/intel_drv.h | 25 +++++--------------------
> drivers/gpu/drm/i915/intel_dvo.c | 7 ++-----
> drivers/gpu/drm/i915/intel_hdmi.c | 10 ++--------
> drivers/gpu/drm/i915/intel_lvds.c | 2 +-
> drivers/gpu/drm/i915/intel_sdvo.c | 14 +++++++-------
> drivers/gpu/drm/i915/intel_tv.c | 2 +-
> 9 files changed, 35 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 9525822..c3f6680 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -690,9 +690,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.clone_mask = (1 << INTEL_SDVO_NON_TV_CLONE_BIT |
> - 1 << INTEL_ANALOG_CLONE_BIT |
> - 1 << INTEL_SDVO_LVDS_CLONE_BIT);
> + crt->base.cloneable = true;
> if (IS_HASWELL(dev))
> crt->base.crtc_mask = (1 << 0);
> else
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1a201b2..38891ff 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6716,15 +6716,23 @@ int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
> return 0;
> }
>
> -static int intel_encoder_clones(struct drm_device *dev, int type_mask)
> +static int intel_encoder_clones(struct intel_encoder *encoder)
> {
> - struct intel_encoder *encoder;
> + struct drm_device *dev = encoder->base.dev;
> + struct intel_encoder *source_encoder;
> int index_mask = 0;
> int entry = 0;
>
> - list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) {
> - if (type_mask & encoder->clone_mask)
> + 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)
> + index_mask |= (1 << entry);
> +
> entry++;
> }
>
> @@ -6883,7 +6891,7 @@ static void intel_setup_outputs(struct drm_device *dev)
> list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) {
> encoder->base.possible_crtcs = encoder->crtc_mask;
> encoder->base.possible_clones =
> - intel_encoder_clones(dev, encoder->clone_mask);
> + intel_encoder_clones(encoder);
> }
>
> /* disable all the possible outputs/crtcs before entering KMS mode */
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 3190e9d..95b1022 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2481,18 +2481,10 @@ intel_dp_init(struct drm_device *dev, int output_reg)
>
> connector->polled = DRM_CONNECTOR_POLL_HPD;
>
> - if (output_reg == DP_B || output_reg == PCH_DP_B)
> - intel_encoder->clone_mask = (1 << INTEL_DP_B_CLONE_BIT);
> - else if (output_reg == DP_C || output_reg == PCH_DP_C)
> - intel_encoder->clone_mask = (1 << INTEL_DP_C_CLONE_BIT);
> - else if (output_reg == DP_D || output_reg == PCH_DP_D)
> - intel_encoder->clone_mask = (1 << INTEL_DP_D_CLONE_BIT);
> + intel_encoder->cloneable = false;
>
> - if (is_edp(intel_dp)) {
> - intel_encoder->clone_mask = (1 << INTEL_EDP_CLONE_BIT);
> - INIT_DELAYED_WORK(&intel_dp->panel_vdd_work,
> - ironlake_panel_vdd_work);
> - }
> + INIT_DELAYED_WORK(&intel_dp->panel_vdd_work,
> + ironlake_panel_vdd_work);
>
> intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 16680e5..f8c1f04 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -89,25 +89,6 @@
> #define INTEL_OUTPUT_DISPLAYPORT 7
> #define INTEL_OUTPUT_EDP 8
>
> -/* Intel Pipe Clone Bit */
> -#define INTEL_HDMIB_CLONE_BIT 1
> -#define INTEL_HDMIC_CLONE_BIT 2
> -#define INTEL_HDMID_CLONE_BIT 3
> -#define INTEL_HDMIE_CLONE_BIT 4
> -#define INTEL_HDMIF_CLONE_BIT 5
> -#define INTEL_SDVO_NON_TV_CLONE_BIT 6
> -#define INTEL_SDVO_TV_CLONE_BIT 7
> -#define INTEL_SDVO_LVDS_CLONE_BIT 8
> -#define INTEL_ANALOG_CLONE_BIT 9
> -#define INTEL_TV_CLONE_BIT 10
> -#define INTEL_DP_B_CLONE_BIT 11
> -#define INTEL_DP_C_CLONE_BIT 12
> -#define INTEL_DP_D_CLONE_BIT 13
> -#define INTEL_LVDS_CLONE_BIT 14
> -#define INTEL_DVO_TMDS_CLONE_BIT 15
> -#define INTEL_DVO_LVDS_CLONE_BIT 16
> -#define INTEL_EDP_CLONE_BIT 17
> -
> #define INTEL_DVO_CHIP_NONE 0
> #define INTEL_DVO_CHIP_LVDS 1
> #define INTEL_DVO_CHIP_TMDS 2
> @@ -153,11 +134,15 @@ struct intel_encoder {
> int type;
> bool needs_tv_clock;
> bool connectors_active;
> + /*
> + * 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;
> void (*hot_plug)(struct intel_encoder *);
> void (*enable)(struct intel_encoder *);
> void (*disable)(struct intel_encoder *);
> int crtc_mask;
> - int clone_mask;
> };
>
> struct intel_connector {
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 756e977..86a3d2b 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -424,17 +424,14 @@ 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->clone_mask =
> - (1 << INTEL_DVO_TMDS_CLONE_BIT) |
> - (1 << INTEL_ANALOG_CLONE_BIT);
> + intel_encoder->cloneable = true;
> 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->clone_mask =
> - (1 << INTEL_DVO_LVDS_CLONE_BIT);
> + intel_encoder->cloneable = false;
> 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 188399f..b01900d 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -959,42 +959,36 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg)
> connector->doublescan_allowed = 0;
> intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>
> + intel_encoder->cloneable = false;
> +
> /* Set up the DDC bus. */
> if (sdvox_reg == SDVOB) {
> - intel_encoder->clone_mask = (1 << INTEL_HDMIB_CLONE_BIT);
> intel_hdmi->ddc_bus = GMBUS_PORT_DPB;
> dev_priv->hotplug_supported_mask |= HDMIB_HOTPLUG_INT_STATUS;
> } else if (sdvox_reg == SDVOC) {
> - intel_encoder->clone_mask = (1 << INTEL_HDMIC_CLONE_BIT);
> intel_hdmi->ddc_bus = GMBUS_PORT_DPC;
> dev_priv->hotplug_supported_mask |= HDMIC_HOTPLUG_INT_STATUS;
> } else if (sdvox_reg == HDMIB) {
> - intel_encoder->clone_mask = (1 << INTEL_HDMID_CLONE_BIT);
> intel_hdmi->ddc_bus = GMBUS_PORT_DPB;
> dev_priv->hotplug_supported_mask |= HDMIB_HOTPLUG_INT_STATUS;
> } else if (sdvox_reg == HDMIC) {
> - intel_encoder->clone_mask = (1 << INTEL_HDMIE_CLONE_BIT);
> intel_hdmi->ddc_bus = GMBUS_PORT_DPC;
> dev_priv->hotplug_supported_mask |= HDMIC_HOTPLUG_INT_STATUS;
> } else if (sdvox_reg == HDMID) {
> - intel_encoder->clone_mask = (1 << INTEL_HDMIF_CLONE_BIT);
> intel_hdmi->ddc_bus = GMBUS_PORT_DPD;
> dev_priv->hotplug_supported_mask |= HDMID_HOTPLUG_INT_STATUS;
> } else if (sdvox_reg == DDI_BUF_CTL(PORT_B)) {
> DRM_DEBUG_DRIVER("LPT: detected output on DDI B\n");
> - intel_encoder->clone_mask = (1 << INTEL_HDMIB_CLONE_BIT);
> intel_hdmi->ddc_bus = GMBUS_PORT_DPB;
> intel_hdmi->ddi_port = PORT_B;
> dev_priv->hotplug_supported_mask |= HDMIB_HOTPLUG_INT_STATUS;
> } else if (sdvox_reg == DDI_BUF_CTL(PORT_C)) {
> DRM_DEBUG_DRIVER("LPT: detected output on DDI C\n");
> - intel_encoder->clone_mask = (1 << INTEL_HDMIC_CLONE_BIT);
> intel_hdmi->ddc_bus = GMBUS_PORT_DPC;
> intel_hdmi->ddi_port = PORT_C;
> dev_priv->hotplug_supported_mask |= HDMIC_HOTPLUG_INT_STATUS;
> } else if (sdvox_reg == DDI_BUF_CTL(PORT_D)) {
> DRM_DEBUG_DRIVER("LPT: detected output on DDI D\n");
> - intel_encoder->clone_mask = (1 << INTEL_HDMID_CLONE_BIT);
> intel_hdmi->ddc_bus = GMBUS_PORT_DPD;
> intel_hdmi->ddi_port = PORT_D;
> dev_priv->hotplug_supported_mask |= HDMID_HOTPLUG_INT_STATUS;
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index f1d0a05..1e879bf 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -943,7 +943,7 @@ bool intel_lvds_init(struct drm_device *dev)
> intel_connector_attach_encoder(intel_connector, intel_encoder);
> intel_encoder->type = INTEL_OUTPUT_LVDS;
>
> - intel_encoder->clone_mask = (1 << INTEL_LVDS_CLONE_BIT);
> + intel_encoder->cloneable = false;
> 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 d630db8..467d92a 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2129,8 +2129,7 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
> connector->connector_type = DRM_MODE_CONNECTOR_HDMIA;
> intel_sdvo->is_hdmi = true;
> }
> - intel_sdvo->base.clone_mask = ((1 << INTEL_SDVO_NON_TV_CLONE_BIT) |
> - (1 << INTEL_ANALOG_CLONE_BIT));
> + intel_sdvo->base.cloneable = true;
>
> intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo);
> if (intel_sdvo->is_hdmi)
> @@ -2161,7 +2160,7 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type)
>
> intel_sdvo->is_tv = true;
> intel_sdvo->base.needs_tv_clock = true;
> - intel_sdvo->base.clone_mask = 1 << INTEL_SDVO_TV_CLONE_BIT;
> + intel_sdvo->base.cloneable = false;
>
> intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo);
>
> @@ -2204,8 +2203,7 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, int device)
> intel_sdvo_connector->output_flag = SDVO_OUTPUT_RGB1;
> }
>
> - intel_sdvo->base.clone_mask = ((1 << INTEL_SDVO_NON_TV_CLONE_BIT) |
> - (1 << INTEL_ANALOG_CLONE_BIT));
> + intel_sdvo->base.cloneable = true;
>
> intel_sdvo_connector_init(intel_sdvo_connector,
> intel_sdvo);
> @@ -2237,8 +2235,10 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
> intel_sdvo_connector->output_flag = SDVO_OUTPUT_LVDS1;
> }
>
> - intel_sdvo->base.clone_mask = ((1 << INTEL_ANALOG_CLONE_BIT) |
> - (1 << INTEL_SDVO_LVDS_CLONE_BIT));
> + /* SDVO LVDS is cloneable because the SDVO encoder does the upscaling,
> + * as opposed to native LVDS, where we upscale with the panel-fitter
> + * (and hence only the native LVDS resolution could be cloned). */
> + intel_sdvo->base.cloneable = true;
>
> intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo);
> if (!intel_sdvo_create_enhance_property(intel_sdvo, intel_sdvo_connector))
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index a0ec1eb..ef9e7a5 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1624,7 +1624,7 @@ 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->clone_mask = (1 << INTEL_TV_CLONE_BIT);
> + intel_encoder->cloneable = false;
> 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;
> --
> 1.7.7.6
>
--
Paulo Zanoni
More information about the Intel-gfx
mailing list