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

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2202 bytes --]

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

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel