Re: [PATCH 1/6] drm/i915: Use atomic state to obtain load detection crtc.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Feb 01, 2016 at 02:43:57PM +0100, Maarten Lankhorst wrote:
> Instead of restoring dpms and a flag for whether a temp fb is allocated duplicate
> the old plane_state and crtc_state, and restore the members we potentially touched.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 128 ++++++++++++++++++++---------------
>  drivers/gpu/drm/i915/intel_drv.h     |   4 +-
>  2 files changed, 76 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4d8c9f7857db..0702ce8ec36a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10361,6 +10361,7 @@ mode_fits_in_fbdev(struct drm_device *dev,
>  	if (obj->base.size < mode->vdisplay * fb->pitches[0])
>  		return NULL;
>  
> +	drm_framebuffer_reference(fb);
>  	return fb;
>  #else
>  	return NULL;
> @@ -10426,6 +10427,9 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
>  		      encoder->base.id, encoder->name);
>  
>  retry:
> +	old->old_pipe_config = NULL;
> +	old->old_plane_state = NULL;
> +
>  	ret = drm_modeset_lock(&config->connection_mutex, ctx);
>  	if (ret)
>  		goto fail;
> @@ -10441,24 +10445,15 @@ retry:
>  	 */
>  
>  	/* See if we already have a CRTC for this connector */
> -	if (encoder->crtc) {
> -		crtc = encoder->crtc;
> +	if (connector->state->crtc) {

All these connector->state accesses made me a bit uneasy, but we did
indeed grab connection_mutex already so it should be fine. It's even
more troubling seeing connector->state accessed outside
intel_get_load_detect_pipe() in the later patches, but it seems it's
only done if intel_get_load_detect_pipe() succeeded which means we
should be holding the right lock.

Dunno, maybe there should be some comments explaining this stuff.
Or maybe maybe we should have a helper to return the current
connector state that also asserts that the right lock is held?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx