On Wed, May 08, 2013 at 12:55:18PM +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.
>
> 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 at linux.intel.com>
> ---
> 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
