diff mbox series

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

Message ID 20210115110626.12233-1-contact@emersion.fr (mailing list archive)
State New, archived
Headers show
Series [v5,1/2] drm/doc: fix drm_plane_type docs | expand

Commit Message

Simon Ser Jan. 15, 2021, 11:06 a.m. UTC
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@emersion.fr>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
---
 include/drm/drm_plane.h | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

Comments

Pekka Paalanen Jan. 18, 2021, 8:40 a.m. UTC | #1
On Fri, 15 Jan 2021 12:06:25 +0100
Simon Ser <contact@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@emersion.fr>
> Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
> Cc: Pekka Paalanen <ppaalanen@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?

>  	 */
>  	DRM_PLANE_TYPE_CURSOR,
>  };

Anyway,
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>


Thanks,
pq
Daniel Vetter Jan. 18, 2021, 9:45 a.m. UTC | #2
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@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@emersion.fr>
> > Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
> > Cc: Pekka Paalanen <ppaalanen@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@collabora.com>
> 
> 
> Thanks,
> pq
diff mbox series

Patch

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.
 	 */
 	DRM_PLANE_TYPE_CURSOR,
 };