[PATCH v5 1/2] drm/doc: fix drm_plane_type docs

[Daniel Vetter]

On Mon, Jan 18, 2021 at 10:40:52AM +0200, Pekka Paalanen wrote:
> On Fri, 15 Jan 2021 12:06:25 +0100
> Simon Ser <contact at emersion.fr> 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 at emersion.fr>
> > Reviewed-by: Daniel Vetter <daniel at ffwll.ch>
> > Cc: Pekka Paalanen <ppaalanen at gmail.com>
> > ---
> >  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 8ef06ee1c8eb..95ab14a4336a 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 pointers for legacy
> > + * IOCTLs. See 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.
> > +	 * A primary plane attached to a CRTC is the most likely to be able to
> > +	 * light up the CRTC when no scaling/cropping is used and the plane
> > +	 * covers the whole CRTC.
> >  	 */
> >  	DRM_PLANE_TYPE_PRIMARY,
> >  
> >  	/**
> >  	 * @DRM_PLANE_TYPE_CURSOR:
> >  	 *
> > -	 * Cursor planes represent a "cursor" plane for a CRTC.  Cursor planes
> > -	 * are the planes operated upon by the DRM_IOCTL_MODE_CURSOR and
> > -	 * DRM_IOCTL_MODE_CURSOR2 IOCTLs.
> > +	 * A cursor plane attached to a CRTC is more likely to be able to be
> > +	 * enabled when no scaling/cropping is used and the framebuffer has the
> > +	 * size indicated by &drm_mode_config.cursor_width and
> > +	 * &drm_mode_config.cursor_height. Additionally, if the driver doesn't
> > +	 * support modifiers, the framebuffer should have a linear layout.
> 
> Hi,
> 
> is there anything to be said about positioning a cursor plane partially
> off-screen?

It should work, like anything partially off-screen placed plane. But
there's two issues:
- you might run into hw limitations (and uh there's even hw where this
  holds for the cursor plane, specifically cursor on 3rd crtc on
  cherrytrail i915 is broken and we can't do some of the placements you'd
  want from a cursor because the display block would die).
- there's still a bunch of drivers which don't even clip correctly :-/

Iow, it's a bit a mess ...
-Daniel

> 
> >  	 */
> >  	DRM_PLANE_TYPE_CURSOR,
> >  };
> 
> Anyway,
> Acked-by: Pekka Paalanen <pekka.paalanen at collabora.com>
> 
> 
> Thanks,
> pq



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch