Hi
I like your idea.
2012/7/11 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.
I don't agree with this sentence... Our documentation contains
sections called "Simultaneous Display Capabilities on a Single Display
Pipe/Transcoder" describing the details and what/how/who/when cloning
is possible.
So in our code, before your patch, these were valid:
- CRT could be cloned with CRT, SDVO-non-tv, SDVO-lvds
- DVO-tmds could be cloned with CRT, DVO-tmds (notice that even
though DVO-tmds can be cloned with CRT, CRT can't be cloned with
DVO-tmds! bug!)
- SDVO-non-tv could be cloned with CRT and SDVO-non-tv
- SDVO-lvds could be cloned with CRT, SDVO-lvds
After your patch, all these can be cloned with each other:
- CRT, DVO-tmds, SDVO-non-tv, SDVO-lvds
Things that were previously not possible and now are possible:
- SDVO-non-tv with SDVO-lvds
- DVO-tmds with SDVO-non-tv (does hardware with both DVO and SDVO exist?)
- DVO-tmds with SDVO-lvds (same question)
Maybe we should find a way to move all this code to inside
intel_encoder_clones directly? Or keep your patch just like it is, but
add a check inside intel_encoder_clones preventing SDVO-non-tv with
SDVO-lvds?
> 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).
>
> Also explain why sdvo LVDS outputs cannot be cloned: Native LVDS (and
I think you mean "sdvo LVDS can be cloned".
> 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);
You removed the "if" statement... Is this correct? Even if it is
correct, move to a separate patch?
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index d630db8..5fe044c 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;
Please add a white space between "cloneable" and "=".
Cheers,
Paulo
--
Paulo Zanoni
