On Mon, Jun 10, 2013 at 02:19:45PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <[email protected]>
>
> In case we detect a "ghost eDP", intel_dp_init_connector was freeing
> both the connector and encoder and then returning. On Haswell,
> intel_ddi_init was trying to use the freed encoder on the HDMI
> initialization path since the following commit:
>
> commit 21a8e6a4853b2ed39fa4c5188a710f2cf1b92026
> Author: Daniel Vetter <[email protected]>
> Date: Wed Apr 10 23:28:35 2013 +0200
> drm/i915: don't setup hdmi for port D edp in ddi_init
>
> So on this patch we change intel_dp_init_connector from void to bool,
> check its return value on its callers and leave the resource-freeing
> task to its callers (since the callers are the ones who allocate the
> resources). With this change it will be easier to see that
> intel_dp_init_connector can fail, so I hope we won't have similar
> regressions in the future.
>
> Also include a small change to remove extra "{" and "}" on nearby
> code.
>
> Signed-off-by: Paulo Zanoni <[email protected]>
Oops, thanks for tracking this down and fixing it. Patch is queued for
-next.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 10 +++++++---
> drivers/gpu/drm/i915/intel_dp.c | 15 +++++++++------
> drivers/gpu/drm/i915/intel_drv.h | 3 ++-
> 3 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 224ce25..4e0cf37 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1356,14 +1356,18 @@ void intel_ddi_init(struct drm_device *dev, enum port
> port)
> intel_encoder->cloneable = false;
> intel_encoder->hot_plug = intel_ddi_hot_plug;
>
> - intel_dp_init_connector(intel_dig_port, dp_connector);
> + /* May fail if it's an eDP connector that's disconnected. */
> + if (!intel_dp_init_connector(intel_dig_port, dp_connector)) {
> + intel_ddi_destroy(encoder);
> + intel_dp_destroy(&dp_connector->base);
> + return;
> + }
>
> if (intel_encoder->type != INTEL_OUTPUT_EDP) {
> hdmi_connector = kzalloc(sizeof(struct intel_connector),
> GFP_KERNEL);
> - if (!hdmi_connector) {
> + if (!hdmi_connector)
> return;
> - }
>
> intel_dig_port->hdmi.hdmi_reg = DDI_BUF_CTL(port);
> intel_hdmi_init_connector(intel_dig_port, hdmi_connector);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0de82bb..ec23b3d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2687,7 +2687,7 @@ done:
> return 0;
> }
>
> -static void
> +void
> intel_dp_destroy(struct drm_connector *connector)
> {
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> @@ -2961,7 +2961,7 @@ intel_dp_init_panel_power_sequencer_registers(struct
> drm_device *dev,
> I915_READ(pp_div_reg));
> }
>
> -void
> +bool
> intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> struct intel_connector *intel_connector)
> {
> @@ -3095,9 +3095,7 @@ intel_dp_init_connector(struct intel_digital_port
> *intel_dig_port,
> } else {
> /* if this fails, presume the device is a ghost */
> DRM_INFO("failed to retrieve link info, disabling
> eDP\n");
> - intel_dp_encoder_destroy(&intel_encoder->base);
> - intel_dp_destroy(connector);
> - return;
> + return false;
> }
>
> /* We now know it's not a ghost, init power sequence regs. */
> @@ -3152,6 +3150,8 @@ intel_dp_init_connector(struct intel_digital_port
> *intel_dig_port,
> u32 temp = I915_READ(PEG_BAND_GAP_DATA);
> I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
> }
> +
> + return true;
> }
>
> void
> @@ -3197,5 +3197,8 @@ intel_dp_init(struct drm_device *dev, int output_reg,
> enum port port)
> intel_encoder->cloneable = false;
> intel_encoder->hot_plug = intel_dp_hot_plug;
>
> - intel_dp_init_connector(intel_dig_port, intel_connector);
> + if (!intel_dp_init_connector(intel_dig_port, intel_connector)) {
> + intel_dp_encoder_destroy(encoder);
> + intel_dp_destroy(&intel_connector->base);
> + }
> }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index b9e250c..8a7ad99 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -569,9 +569,10 @@ extern bool intel_lvds_init(struct drm_device *dev);
> extern bool intel_is_dual_link_lvds(struct drm_device *dev);
> extern void intel_dp_init(struct drm_device *dev, int output_reg,
> enum port port);
> -extern void intel_dp_init_connector(struct intel_digital_port
> *intel_dig_port,
> +extern bool intel_dp_init_connector(struct intel_digital_port
> *intel_dig_port,
> struct intel_connector *intel_connector);
> extern void intel_dp_init_link_config(struct intel_dp *intel_dp);
> +extern void intel_dp_destroy(struct drm_connector *connector);
> extern void intel_dp_start_link_train(struct intel_dp *intel_dp);
> extern void intel_dp_complete_link_train(struct intel_dp *intel_dp);
> extern void intel_dp_stop_link_train(struct intel_dp *intel_dp);
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
