Re: [PATCH] drm: rework description of primary and cursor planes

From: Daniel Vetter <daniel@ffwll.ch>
To: Simon Ser <contact@emersion.fr>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm: rework description of primary and cursor planes
Date: Wed, 9 Dec 2020 17:02:23 +0100	[thread overview]
Message-ID: <20201209160223.GT401619@phenom.ffwll.local> (raw)
In-Reply-To: <0zedd9O9Bp0DfEH26xBTGvZtqA5bdE2EJDN3z5TXiDIyiwfTnRapgDy69MjAlhMWrzqKTzoYwovpGANNhp1PmneSCrm-xzw9DIeauv1SkgM=@emersion.fr>

On Wed, Dec 09, 2020 at 03:58:17PM +0000, Simon Ser wrote:
> Thanks for the review!
> 
> On Wednesday, December 9th, 2020 at 1:42 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > I think maybe a follow up patch should document how userspace should
> > figure out how to line up the primary planes with the right crtcs (if it
> > wishes to know that information, it's not super useful aside from probably
> > good choice for a fullscreen fallback plane). See my reply on the old
> > thread.
> 
> Yeah, as I've already replied, I won't volunteer to document legacy bits,
> documenting atomic is already enough. :P

This is also somewhat useful as a hint for atomic userspace.

> > And that patch should also add the code to drm_mode_config_validate() to
> > validate the possible_crtc masks for these. Something like
> >
> > 	num_primary = 0; num_cursor = 0;
> >
> > 	for_each_plane(plane) {
> > 		if (plane->type == primary) {
> > 			WARN_ON(!(plane->possible_crtcs & BIT(num_primary)));
> > 			num_primary++;
> > 		}
> >
> > 		/* same for cursor */
> > 	}
> >
> > 	WARN_ON(num_primary != dev->mode_config.num_crtcs);
> > }
> 
> Thanks for the suggestion. However I don't see why a driver should ensure
> this. Isn't it perfectly fine for a driver to use primary plane 2 for CRTC 1,
> and primary plane 1 for CRTC 2, for the purposes of legacy uAPI?

Yeah but it's a mess. Messes don't make good uapi.

> If we're trying here to invent a new uAPI to let user-space discover the
> internal legacy primary/cursor pointers, I'm not signing up for this. The
> requirement you're describing seems like something current drivers ensure
> "by chance", not an established uAPI. In other words, writing a new driver
> that doesn't do the same wouldn't break uAPI contracts.

I'm more seeing this as general uapi lock-down. "Everything goes" doesn't
work great. And it's all the same topic really. Like if your userspace
really doesn't care about what the primary plane is (like you seem to
imply), then ofc none of this matters to you, but then also your doc patch
here doesn't matter.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel