Re: [PATCH 6/6] drm/i915: Switch to unified plane cursor handling (v4)

[Matt Roper <matthew.d.roper@intel.com>]

On Thu, Jun 12, 2014 at 04:47:18AM -0700, G, Pallavi wrote:
> On Tue, 2014-06-10 at 08:28 -0700, Matt Roper wrote:
...
> > @@ -8885,8 +8857,6 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
> >  		kfree(work);
> >  	}
> >  
> > -	intel_crtc_cursor_set(crtc, NULL, 0, 0, 0)
> 
>         Please help me to understand how the cursor enable/disable will
> handled in the legacy path if we remove the cursor disable from
> intel_crtc_destroy

Good question.  When the driver is shutting down the DRM core tears down
all the KMS stuff associated with the driver.  One of those steps is
destroying the CRTC which, as you note, previously would take care of
turning off the cursor plane.  However now that the cursor exists as its
own drm_plane, and whatever is being scanned out by the cursor is a real
drm_framebuffer that the DRM core knows about, the cursor's FB should
get destroyed by the DRM core, which will trigger the cursor disable
entrypoint on the driver.  The call sequence is

drm_framebuffer_remove() -> drm_force_plane_disable()
                         -> intel_cursor_plane_disable()


> > -
> >  	drm_crtc_cleanup(crtc);
> >  
> >  	kfree(intel_crtc);
> > @@ -10942,8 +10912,6 @@ out_config:
> >  }
> >  
> >  static const struct drm_crtc_funcs intel_crtc_funcs = {
> > -	.cursor_set = intel_crtc_cursor_set,
> > -	.cursor_move = intel_crtc_cursor_move,
> 
> I don't find the corresponding changes in the drm layer related to the
> cursor_set and cursor_move removal. Even in the patch 3 only for the
> universal plane drm_mode_cursor_universal is called what about the
> legacy path?

Right, the driver entrypoints remain in the drm_crtc_funcs structure
because other non-i915 drivers may not have implemented universal cursor
planes, so the DRM core will still call into their .cursor_set and
.cursor_move entrypoints when they issue a legacy cursor ioctl.  We
don't want to force other driver authors to update to universal planes
until they're ready, so we want to keep the old code paths working until
everyone has updated.

Drivers like i915 that do get updated to have universal cursor planes
will no longer receive calls into these legacy entrypoints anymore
(since everything will come into the universal entrypoint), so there's
no need for us to keep the legacy .cursor_set and .cursor_move around in
our own driver.


Matt

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795