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

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

On Mon, Jun 03, 2013 at 02:00:00PM +0200, Daniel Vetter wrote:
> On Fri, May 31, 2013 at 08:07:03PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@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@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.

OK I give up. I did a quick check of the private plane stuff, and it
turns out they're not on plane_list, so I think just walking the list
and disabling everything there should be OK. Since you're happy w/
leaving the video overlay out in the cold for now, I'll shovel it all
into drm_fb_helper...


-- 
Ville Syrjälä
Intel OTC