[Intel-gfx] [PATCH v2 3/7] drm/i915: Fix fbdev sprite disable code

[Daniel Vetter]

On Fri, May 31, 2013 at 08:07:03PM +0300, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> plane->enabled is never set, so this code didn't do anything.
> 
> Fix the code for sprites by calling the new drm_plane_force_disable()
> function. That means the plane will remain off until someone explicitly
> turns it back on.
> 
> And do the same for cursors and the old video overlays, since we only
> want to see the primary plane for fbdev.
> 
> v2: Disable sprites/cursors until explicitly re-enabled
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_fb.c      |  9 ++-------
>  3 files changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 944b6d5..1d1a3fd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9842,3 +9842,19 @@ intel_display_print_error_state(struct drm_i915_error_state_buf *m,
>  	}
>  }
>  #endif
> +
> +void intel_disable_cursors_and_sprites(struct drm_device *dev)
> +{
> +	struct drm_crtc *crtc;
> +	struct drm_plane *plane;
> +
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +		intel_crtc_dpms_overlay(to_intel_crtc(crtc), false);
> +		intel_crtc_cursor_set(crtc, NULL, 0, 0, 0);
> +	}

I still think we should just move this into drm_fb_helper_restore_fbdev_mode:
- Cursors can be killed with crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0);
  Or at least we should be able to do so, I've done a quick audit of all
  drivers and none really cares about file_priv if the handle is 0. Of
  course we should put that into a drm_cursor_force_disable helper to
  prepare for a better world (atomic modeset and all ...).

  vmwgfx needs to be fixed up slightly since it derefences file_priv
  outside of a handle != 0 check, but that one can be trivially fixed for
  both cursor_set callbacks by moving the driver_private dereference into
  the if block.

- Legacy overlay stuff has been broken like that forever, and at least on
  i8xx those are used much more often. Imo whoever's offended by gen2
  being a bit broken can write a drm plane wrapper for the overlay stuff
  (or better, move the code to drm planes and convert the ioctl to be a
  shim around the real drm plane interfaces). I don't think we should add
  hacks for that.

Cheers, Daniel

> +
> +	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
> +		if (plane->fb)
> +			drm_plane_force_disable(plane);
> +	}
> +}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d0a8c1a..f8e76cf 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -800,5 +800,6 @@ extern bool intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev,
>  extern bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev,
>  						 enum transcoder pch_transcoder,
>  						 bool enable);
> +extern void intel_disable_cursors_and_sprites(struct drm_device *dev);
>  
>  #endif /* __INTEL_DRV_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index 5fe525a..b34ccf3 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -293,22 +293,17 @@ void intel_fb_restore_mode(struct drm_device *dev)
>  {
>  	int ret;
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> -	struct drm_mode_config *config = &dev->mode_config;
> -	struct drm_plane *plane;
>  
>  	if (INTEL_INFO(dev)->num_pipes == 0)
>  		return;
>  
>  	drm_modeset_lock_all(dev);
>  
> +	intel_disable_cursors_and_sprites(dev);
> +
>  	ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper);
>  	if (ret)
>  		DRM_DEBUG("failed to restore crtc mode\n");
>  
> -	/* Be sure to shut off any planes that may be active */
> -	list_for_each_entry(plane, &config->plane_list, head)
> -		if (plane->enabled)
> -			plane->funcs->disable_plane(plane);
> -
>  	drm_modeset_unlock_all(dev);
>  }
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch