On Thu, May 15, 2014 at 12:10:16PM +0200, Daniel Vetter wrote:
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 think you misunderstood what I was saying. What I proposed wasn't that drm_vblank_get() was replaced by a new implementation with different signature. Rather my suggestion was to implement the old drm_vblank_get() by calling drm_crtc_vblank_get() rather than the other way around.
Something like this:
int drm_crtc_vblank_get(struct drm_crtc *crtc) { new code using CRTC }
int drm_vblank_get(struct drm_device *drm, int crtc) { struct drm_crtc *c = drm_crtc_from_index(crtc);
return drm_crtc_vblank_get(c); }
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.
On that note, is there a plan to move the vblank fields out of the DRM device and into drm_crtc as well? That seems like a logical next step since presumably every CRTC can handle it's own vblank events itself.
Thierry