Re: [PATCH] drm/i915: duct-tape locking when eDP init fails

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 25 Mar 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Thanks to apple gpu mux fail we detect an eDP output, but can't read
> anything over dp aux. In the resulting failure path we then hit a
> paranoid WARN about potential locking.
>
> Since the WARN is pretty useful for normal operation just paper over
> it in the failure case by grabbing the demanded (but for init/teardown
> not really required) lock.
>
> I've checked our driver unload code and we already don't hold the kms
> lock when calling drm_mode_config_cleanup. So this won't lead to a new
> deadlock when reloading i915.ko.

Also, drm_encoder_cleanup() grabs mode_config mutex internally, so we
would have deadlocks already if intel_dp_encoder_destroy() were called
while holding the lock.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


An observation outside of this patch, I find it a bit ugly that it
depends on the ironlake_edp_panel_vdd_off() sync parameter whether the
caller needs to hold the mode_config mutex or not. Perhaps separate
functions for sync/async would be neater, but don't hold your breath
waiting for patches from me to that effect. ;)

>
> v2: Make it compile.
>
> Reported-by: Dave Airlie <airlied@gmail.com>
> Cc: Dave Airlie <airlied@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |    3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d7d4afe..8fc93f9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2559,12 +2559,15 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
>  {
>  	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
>  	struct intel_dp *intel_dp = &intel_dig_port->dp;
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>  
>  	i2c_del_adapter(&intel_dp->adapter);
>  	drm_encoder_cleanup(encoder);
>  	if (is_edp(intel_dp)) {
>  		cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
> +		mutex_lock(&dev->mode_config.mutex);
>  		ironlake_panel_vdd_off_sync(intel_dp);
> +		mutex_unlock(&dev->mode_config.mutex);
>  	}
>  	kfree(intel_dig_port);
>  }
> -- 
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx