Received-SPF: pass (google.com: domain of linux-kernel+bounces-145329-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Mon, 15 Apr 2024 21:16:18 +0700
From: Bagas Sanjaya <bagasdotme@gmail.com>
To: Louis Chauvet <louis.chauvet@bootlin.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	seanpaul@google.com, marcheu@google.com, nicolejadeyee@google.com,
	pekka.paalanen@collabora.com, thomas.petazzoni@bootlin.com
Subject: Re: [PATCH 3/3] drm/fourcc: Add documentation around drm_format_info
Message-ID: <Zh02sk_NAy6eMSCt@archie.me>
References: <20240409-google-drm-doc-v1-0-033d55cc8250@bootlin.com>
 <20240409-google-drm-doc-v1-3-033d55cc8250@bootlin.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="/AhLcWdqLXTIlsLz"
Content-Disposition: inline
In-Reply-To: <20240409-google-drm-doc-v1-3-033d55cc8250@bootlin.com>


--/AhLcWdqLXTIlsLz
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Apr 09, 2024 at 12:04:07PM +0200, Louis Chauvet wrote:
>  /**
>   * struct drm_format_info - information about a DRM format
> + *
> + * A drm_format_info describes how planes and pixels are stored in memor=
y.
> + *
> + * Some format like YUV can have multiple planes, counted in @num_planes=
=2E It
> + * means that a full pixel can be stored in multiple non-continuous buff=
ers.
> + * For example, NV12 is a YUV format using two planes: one for the Y val=
ues and
> + * one for the UV values.
> + *
> + * On each plane, the "pixel" unit can be different in case of subsampli=
ng. For
> + * example with the NV12 format, a pixel in the UV plane is used for fou=
r pixels
> + * in the Y plane.
> + * The fields @hsub and @vsub are the relation between the size of the m=
ain
> + * plane and the size of the subsampled planes in pixels:
> + *	plane[0] width =3D hsub * plane[1] width
> + *	plane[0] height =3D vsub * plane[1] height
> + *
> + * In some formats, pixels are not independent in memory. It can be a pa=
cked
> + * representation to store more pixels per byte (for example P030 uses 4=
 bytes
> + * for three 10 bit pixels). It can also be used to represent tiled form=
ats,
> + * where a continuous buffer in memory can represent a rectangle of pixe=
ls (for
> + * example, in DRM_FORMAT_Y0L0, a buffer of 8 bytes represents a 2x2 pix=
el
> + * region of the picture).
> + *	The field @char_per_block is the size of a block on a specific plane,=
 in
> + *	bytes.
> + *	The fields @block_w and @block_h are the size of a block in pixels.
> + *
> + * The older format representation (which only uses @cpp, kept for histo=
rical
> + * reasons because there are a lot of places in drivers where it's used)=
 is
> + * assuming that a block is always 1x1 pixel.
> + *
> + * To keep the compatibility with older format representations and treat=
 block
> + * and non-block formats in the same way one should use:
> + *	- @char_per_block to access the size of a block on a specific plane, =
in
> + *	bytes.
> + *	- drm_format_info_block_width() to access the width of a block of a
> + *	specific plane, in pixels.
> + *	- drm_format_info_block_height() to access the height of a block of a
> + *	specific plane, in pixels.
>   */
>  struct drm_format_info {
>  	/** @format: 4CC format identifier (DRM_FORMAT_*) */
> @@ -97,13 +135,6 @@ struct drm_format_info {
>  		 * formats for which the memory needed for a single pixel is not
>  		 * byte aligned.
>  		 *
> -		 * @cpp has been kept for historical reasons because there are
> -		 * a lot of places in drivers where it's used. In drm core for
> -		 * generic code paths the preferred way is to use
> -		 * @char_per_block, drm_format_info_block_width() and
> -		 * drm_format_info_block_height() which allows handling both
> -		 * block and non-block formats in the same way.
> -		 *
>  		 * For formats that are intended to be used only with non-linear
>  		 * modifiers both @cpp and @char_per_block must be 0 in the
>  		 * generic format table. Drivers could supply accurate
>=20

Sphinx reports htmldocs warnings:

Documentation/gpu/drm-kms:357: ./include/drm/drm_fourcc.h:74: ERROR: Unexpe=
cted indentation.
Documentation/gpu/drm-kms:357: ./include/drm/drm_fourcc.h:83: ERROR: Unexpe=
cted indentation.
Documentation/gpu/drm-kms:357: ./include/drm/drm_fourcc.h:93: ERROR: Unexpe=
cted indentation.
Documentation/gpu/drm-kms:357: ./include/drm/drm_fourcc.h:94: WARNING: Bull=
et list ends without a blank line; unexpected unindent.

I have to fix up the lists:

---- >8 ----
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 66cc30e28f794a..10ee74fa46d21e 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -71,8 +71,9 @@ struct drm_mode_fb_cmd2;
  * in the Y plane.
  * The fields @hsub and @vsub are the relation between the size of the main
  * plane and the size of the subsampled planes in pixels:
- *	plane[0] width =3D hsub * plane[1] width
- *	plane[0] height =3D vsub * plane[1] height
+ *
+ *	- plane[0] width =3D hsub * plane[1] width
+ *	- plane[0] height =3D vsub * plane[1] height
  *
  * In some formats, pixels are not independent in memory. It can be a pack=
ed
  * representation to store more pixels per byte (for example P030 uses 4 b=
ytes
@@ -80,9 +81,10 @@ struct drm_mode_fb_cmd2;
  * where a continuous buffer in memory can represent a rectangle of pixels=
 (for
  * example, in DRM_FORMAT_Y0L0, a buffer of 8 bytes represents a 2x2 pixel
  * region of the picture).
- *	The field @char_per_block is the size of a block on a specific plane, in
- *	bytes.
- *	The fields @block_w and @block_h are the size of a block in pixels.
+ *
+ *	- The field @char_per_block is the size of a block on a specific plane,
+ *	  in bytes.
+ *	- The fields @block_w and @block_h are the size of a block in pixels.
  *
  * The older format representation (which only uses @cpp, kept for histori=
cal
  * reasons because there are a lot of places in drivers where it's used) is
@@ -90,12 +92,13 @@ struct drm_mode_fb_cmd2;
  *
  * To keep the compatibility with older format representations and treat b=
lock
  * and non-block formats in the same way one should use:
+ *
  *	- @char_per_block to access the size of a block on a specific plane, in
- *	bytes.
+ *	  bytes.
  *	- drm_format_info_block_width() to access the width of a block of a
- *	specific plane, in pixels.
+ *	  specific plane, in pixels.
  *	- drm_format_info_block_height() to access the height of a block of a
- *	specific plane, in pixels.
+ *	  specific plane, in pixels.
  */
 struct drm_format_info {
 	/** @format: 4CC format identifier (DRM_FORMAT_*) */

Thanks.

--=20
An old man doll... just what I always wanted! - Clara

--/AhLcWdqLXTIlsLz
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQSSYQ6Cy7oyFNCHrUH2uYlJVVFOowUCZh02rgAKCRD2uYlJVVFO
o/AIAP9nAjuvp2Syyy0KsbDw+mAbk2nVyjWPoMzlKnG+nYAn3gD/fAP+e2p93os3
hY0n5qJZp+AlZtoBCoICJkxphd2UCQM=
=NyHa
-----END PGP SIGNATURE-----

--/AhLcWdqLXTIlsLz--