Re: [PATCH 3/7] drm/i915: Fix fbdev sprite disable code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, May 08, 2013 at 12:55:18PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> 
> plane->enabled is never set, so this code didn't do anything.
> 
> If we end up doing a full modeset, sprites already get disabled. However
> if we end up doing a simple set_base, we still need to turn off the sprites
> manually.
> 
> And do the same for cursors, since we only want to see the primary plane
> for fbdev.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_fb.c      |  8 ++------
>  3 files changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cfe2803..cef0bff 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9928,3 +9928,17 @@ intel_display_print_error_state(struct seq_file *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_update_cursor(crtc, false);
> +	}
> +
> +	list_for_each_entry(plane, &dev->mode_config.plane_list, head)
> +		intel_plane_disable(plane);

Somehow my tree doesn't have an intel_plane_disable and while hunting that
one down I've started to think a bit more ...

Why exactly do we need special driver callbacks for all this? Strictly
speaking fbcon is just another kms client, if it needs to dig around in
driver details that just means our interfaces are not good enough. I know
that there's some historical baggage left around, but for disabling
cursors and planes I don't see any need at all:

Can't we just loop over all crtcs and disable every cursor and then loop
over all sprites and also shut them all down when restoring the fbdev
mode? All without any new driver callbacks?

It might flicker like mad since we can't do it atomically, but then again
this would give us a neat in-kernel user for the atomic modeset stuff
right away ...

Cheers, Daniel

> +}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index cd1297e..a4e866a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -789,5 +789,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 a04481f..57a082c 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -281,8 +281,6 @@ 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;
> @@ -293,10 +291,8 @@ void intel_fb_restore_mode(struct drm_device *dev)
>  	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);
> +	/* in case we ended up doing just set_base above */
> +	intel_disable_cursors_and_sprites(dev);
>  
>  	drm_modeset_unlock_all(dev);
>  }
> -- 
> 1.8.1.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux