Re: [PATCH 08/12] drm/irq: Add kms-native crtc interface functions

Daniel Vetter
2014-05-15T10:10:17Z

On Thu, May 15, 2014 at 9:34 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> This seems slightly backwards. Since drm_vblank_get() is what's being
> deprecated here, wouldn't it make more sense to write
> drm_crtc_vblank_get() in terms of struct drm_crtc and make
> drm_vblank_get() call that instead? I can't seem to find a helper to get
> the CRTC from an index, but it seems like that wouldn't be hard to do.

Two reasons against this:
- More ugly churn since it's a flag day, and when handling vblank
refactorings what I _definitely_ want to avoid is whole-subsystem wide
flag days.
- drm_crtc_ is the common prefix used by many of the crtc functions.

What I actually forgotten to do is drop the dev parameter, we can fish
that out of the crtc. Then it should be even more obvious that this is
a crtc function and rightfully deserve the drm_crtc_ prefix ;-)

> I guess it doesn't matter all that much either way, though, since we
> could equally well make that change when drm_vblank_get() is dropped, in
> which case at least there's no longer a need for the reverse lookup.

Yeah, the reverse lookup is something I want to add later on
eventually. But that requires more thought since it only makes sense
if we also switch the driver callbacks for vblank_enable/disable over.

> I'd still prefer to have i915 changes in a separate commit, but
> otherwise:

Will do, makes indeed more sense.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch