On Thu, Dec 17, 2020 at 11:37 AM Simon Ser <contact@xxxxxxxxxxx> wrote:
>
> On Wednesday, December 16th, 2020 at 10:19 PM, Daniel Vetter <daniel@xxxxxxxx> wrote:
>
> > On Wed, Dec 16, 2020 at 09:22:21PM +0100, Simon Ser wrote:
> > > The docs for enum drm_plane_type mention legacy IOCTLs, however the
> > > plane type is not tied to legacy IOCTLs, the drm_cursor.primary and
> > > cursor fields are. Add a small paragraph to reference these.
> > >
> > > Instead, document expectations for primary and cursor planes for
> > > non-legacy userspace. Note that these docs are for driver developers,
> > > not userspace developers, so internal kernel APIs are mentionned.
> > >
> > > Signed-off-by: Simon Ser <contact@xxxxxxxxxxx>
> > > Cc: Daniel Vetter <daniel@xxxxxxxx>
> > > Cc: Pekka Paalanen <ppaalanen@xxxxxxxxx>
> > > ---
> > > include/drm/drm_plane.h | 21 +++++++++++++--------
> > > 1 file changed, 13 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> > > index 1d82b264e5e4..94fdd0c68450 100644
> > > --- a/include/drm/drm_plane.h
> > > +++ b/include/drm/drm_plane.h
> > > @@ -538,10 +538,14 @@ struct drm_plane_funcs {
> > > *
> > > * For compatibility with legacy userspace, only overlay planes are made
> > > * available to userspace by default. Userspace clients may set the
> > > - * DRM_CLIENT_CAP_UNIVERSAL_PLANES client capability bit to indicate that they
> > > + * &DRM_CLIENT_CAP_UNIVERSAL_PLANES client capability bit to indicate that they
> > > * wish to receive a universal plane list containing all plane types. See also
> > > * drm_for_each_legacy_plane().
> > > *
> > > + * In addition to setting each plane's type, drivers need to setup the
> > > + * &drm_crtc.primary and optionally &drm_crtc.cursor for legacy IOCTLs. See
> >
> > insert "pointers" here ^ for readability.
> >
> > > + * drm_crtc_init_with_planes().
> > > + *
> > > * WARNING: The values of this enum is UABI since they're exposed in the "type"
> > > * property.
> > > */
> > > @@ -557,19 +561,20 @@ enum drm_plane_type {
> > > /**
> > > * @DRM_PLANE_TYPE_PRIMARY:
> > > *
> > > - * Primary planes represent a "main" plane for a CRTC. Primary planes
> > > - * are the planes operated upon by CRTC modesetting and flipping
> > > - * operations described in the &drm_crtc_funcs.page_flip and
> > > - * &drm_crtc_funcs.set_config hooks.
> >
> > I think we should keep the notice which legacy entry hooks implicitly
> > operate on the primary plane. Not sure why you're removing the above
> > sentence. Maybe improve it by adding "See also &drm_crtc.primary." for
> > more context?
>
> I intentionally removed it, because setting the plane type will not magically
> make it used for legacy IOCTLs. The previous version documented the legacy
> IOCTLs behaviour in the primary and cursor type docs. That was misleading
> because only the drm_crtc.{primary,cursor} pointers make it so a plane is used
> for legacy IOCTLs, not the type.
>
> This patch does keep a reference to drm_crtc.{primary,cursor}, in the paragraph
> right above. Clicking on the reference will explain in detail which legacy
> IOCTLs are affected. I don't think repeating the paragraph again in the primary
> and cursor type docs is necessary.
Hm I guess works too, I'm not sure documentations must avoid
repetitions at all costs (since it generally makes stuff harder to
find, despite all the links). So without that change:
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
