On Thu, 2014-06-12 at 07:57 -0700, Matt Roper wrote:
> 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
>
Thanks for the clarification.
with this my doubts are clear. I reviewed the entire patch series with
respect to the legacy support as well as the universal cursor plane
support in general. I dont find any issues and design gaps. I hope Matt
is working on the igt based test for the universal plane support (seen
some patches related to that in the mail thread).
Reviewed-by: Pallavi G <pallavi.g@xxxxxxxxx>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
