[PATCH v2 00/43] drm/sun4i: Support for linear and tiled YUV formats with the frontend

[PATCH v2 00/43] drm/sun4i: Support for linear and tiled YUV formats with the frontend

[Paul Kocialkowski]

This series implements support for YUV formats using the display engine
frontend in the sun4i DRM driver, with various fixes along the way.
Scaling is supported for every format handled by the frontend.

The tiling mode used by the VPU on Allwinner platforms is also supported
by this series and a dedicated fourcc modifier is introduced, along with
a specific ioctl for allocating tiled buffers.

New common fourcc helpers are also introduced in this series, especially
related to YUV formats.

This was tested on the A33 and A20 platforms and all supported features
work properly on both. Framebuffer offsets and source positions are not
supported at this point.

Changes since v1:
* Extended source selected cleanup to both YUV and video channels;
* Split and reworded comment about backend scaling;
* Sorted newly-introduced lists alphabetically;
* Simplified functions to check if a format is supported;
* Reworked frontend selection logic as discussed;
* Made YUV helpers common, with fourcc and drm_info fashions;
* Changed the CREATE_TILED ioctl flags to match the CREATE_DUMB ioctl;
* Reworked YUV dimensions calculations to use drm_format_info;
* Dropped MB32 part in modifier to match V4L2 definition name;
* Improved the description of the tiling modifier;
* Avoided splitting CSC coefficients to avoid introducing a new module;
* Fixed building the driver as modules;
* Various code simplifications and cleanups;
* Split commits into logical changes.

Maxime Ripard (5):
  drm/sun4i: Move access control before setting the register as
    documented
  drm/sun4i: frontend: Add a quirk structure
  drm/sun4i: Set the coef_rdy bit right after the coef have been set
  drm/sun4i: Make COEF_RDY conditional
  drm/sun4i: frontend: Move the FIR filter phases to our quirks

Paul Kocialkowski (38):
  drm/sun4i: Cleanup video/YUV source before enabling a layer
  drm/sun4i: frontend: Replace ARGB with XRGB as supported format
  drm/sun4i: Add TODO comment about supporting scaling with the backend
  drm/sun4i: backend: Add a helper and a list for supported formats
  drm/sun4i: frontend: Add a helper and a list for supported formats
  drm/sun4i: backend: Refine the logic behind using the frontend
  drm/sun4i: backend: Use a specific function to check if a plane is
    supported
  drm/fourcc: Add helper to check if a format uses a YUV colorspace
  drm/fourcc: Add format info helpers for checking YUV planes
    disposition
  drm/fourcc: Add format helpers for checking YUV planes disposition
  drm/fourcc: Add format info helpers for checking YUV sub-sampling
  drm/fourcc: Add format helpers for checking YUV sub-sampling
  drm/sun4i: backend: Use explicit fourcc helpers for packed YUV422
    check
  drm/sun4i: backend: Avoid counting YUV planes that use the frontend
  drm/sun4i: Rename sun4i_backend_layer_formats to sun4i_layer_formats
  drm/sun4i: frontend: Move CSC bypass setup to format update routine
  drm/sun4i: frontend: Add helpers for input data mode and pixel
    sequence
  drm/sun4i: frontend: Add proper definitions for format registers
  drm/sun4i: frontend: Determine input mode based on the number of
    planes
  drm/sun4i: frontend: Determine input format based on colorspace
  drm/sun4i: frontend: Add support for the BGRX8888 input format
  drm/sun4i: frontend: Add support for the BGRX8888 output format
  drm/sun4i: backend: Detail the YUV to RGB values coding explanation
  drm/sun4i: frontend: Configure and enable YUV to RGB CSC when needed
  drm/sun4i: frontend: Apply format sub-sampling to CH1 dimensions
  drm/sun4i: frontend: Add support for packed YUV422 input formats
  drm/sun4i: frontend: Add support for semi-planar YUV input formats
  drm/sun4i: frontend: Add support for planar YUV input formats
  drm/sun4i: Make pitch even for GEM dumb alloc as per hardware
    constraint
  drm/fourcc: Add definitions for Allwinner vendor and VPU tiled format
  drm/sun4i: Add a dedicated ioctl call for allocating tiled buffers
  drm/sun4i: Pass modifier to backend and frontend format support
    helpers
  drm/sun4i: frontend: Add support for tiled YUV input mode
    configuration
  drm/sun4i: Add buffer stride and offset configuration for tiling mode
  drm/sun4i: frontend: Add and use helper for checking tiling support
  drm/sun4i: layer: Add tiled modifier support and helper
  drm/sun4i: drv: Allow framebuffer modifiers in mode config
  drm/sun4i: frontend: Add A20-specific device-tree compatible and
    quirks

 drivers/gpu/drm/drm_fourcc.c           | 184 +++++++++++
 drivers/gpu/drm/sun4i/sun4i_backend.c  | 109 ++++++-
 drivers/gpu/drm/sun4i/sun4i_backend.h  |   3 +
 drivers/gpu/drm/sun4i/sun4i_drv.c      | 102 +++++-
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 415 ++++++++++++++++++++++---
 drivers/gpu/drm/sun4i/sun4i_frontend.h |  50 ++-
 drivers/gpu/drm/sun4i/sun4i_layer.c    |  41 ++-
 include/drm/drm_fourcc.h               | 126 ++++++++
 include/uapi/drm/drm_fourcc.h          |  16 +
 include/uapi/drm/sun4i_drm.h           |  42 +++
 10 files changed, 1029 insertions(+), 59 deletions(-)
 create mode 100644 include/uapi/drm/sun4i_drm.h

-- 
2.19.1

[PATCH v2 01/43] drm/sun4i: Cleanup video/YUV source before enabling a layer

[Paul Kocialkowski]

This adds a dedicated function for cleaning the video and YUV source
channel layer enable bits. This function is called first on layer atomic
update to make sure that there are no leftover bits from previous
plane configuration that were not cleaned until now.

It fixes issues when alternating between video and YUV planes, where
both bits would be set eventually, leading to broken plane display.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 9 +++++++++
 drivers/gpu/drm/sun4i/sun4i_backend.h | 2 ++
 drivers/gpu/drm/sun4i/sun4i_layer.c   | 2 ++
 3 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index bf49c55b0f2c..67b4bb4f5365 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -395,6 +395,15 @@ int sun4i_backend_update_layer_zpos(struct sun4i_backend *backend, int layer,
 	return 0;
 }
 
+void sun4i_backend_cleanup_layer(struct sun4i_backend *backend,
+				 int layer)
+{
+	regmap_update_bits(backend->engine.regs,
+			   SUN4I_BACKEND_ATTCTL_REG0(layer),
+			   SUN4I_BACKEND_ATTCTL_REG0_LAY_VDOEN |
+			   SUN4I_BACKEND_ATTCTL_REG0_LAY_YUVEN, 0);
+}
+
 static bool sun4i_backend_plane_uses_scaler(struct drm_plane_state *state)
 {
 	u16 src_h = state->src_h >> 16;
diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
index e3d4c6035eb2..339dbff1cce4 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
@@ -208,5 +208,7 @@ int sun4i_backend_update_layer_frontend(struct sun4i_backend *backend,
 					int layer, uint32_t in_fmt);
 int sun4i_backend_update_layer_zpos(struct sun4i_backend *backend,
 				    int layer, struct drm_plane *plane);
+void sun4i_backend_cleanup_layer(struct sun4i_backend *backend,
+				 int layer);
 
 #endif /* _SUN4I_BACKEND_H_ */
diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index 78f77af8805a..e0e37c2f1721 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -92,6 +92,8 @@ static void sun4i_backend_layer_atomic_update(struct drm_plane *plane,
 	struct sun4i_backend *backend = layer->backend;
 	struct sun4i_frontend *frontend = backend->frontend;
 
+	sun4i_backend_cleanup_layer(backend, layer->id);
+
 	if (layer_state->uses_frontend) {
 		sun4i_frontend_init(frontend);
 		sun4i_frontend_update_coord(frontend, plane);
-- 
2.19.1

Re: [PATCH v2 01/43] drm/sun4i: Cleanup video/YUV source before enabling a layer

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 679 bytes --]

On Fri, Nov 23, 2018 at 10:24:33AM +0100, Paul Kocialkowski wrote:
> This adds a dedicated function for cleaning the video and YUV source
> channel layer enable bits. This function is called first on layer atomic
> update to make sure that there are no leftover bits from previous
> plane configuration that were not cleaned until now.
> 
> It fixes issues when alternating between video and YUV planes, where
> both bits would be set eventually, leading to broken plane display.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 02/43] drm/sun4i: frontend: Replace ARGB with XRGB as supported format

[Paul Kocialkowski]

The frontend documentation (for the A33) mentions that ARGB is supported
as output, but with the alpha component always set to 0xff. In practice,
this means that the alpha component cannot be preserved when going
through the frontend. Since the information is lost, ARGB is not
properly supported.

As a result, expose the matching format supported by the frontend (both
for input and output) as XRGB instead of ARGB.

Since ARGB was the selected format for connecting the frontend to the
backend, change it to XRGB to reflect this as well.

The A31 and A80 SoCs apparently have a bit to enable proper alpha,
but this is not supported at this point (see the comment already in the
code).

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 3 +--
 drivers/gpu/drm/sun4i/sun4i_layer.c    | 4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index ddf6cfa6dd23..3ea925584891 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -107,7 +107,7 @@ EXPORT_SYMBOL(sun4i_frontend_update_buffer);
 static int sun4i_frontend_drm_format_to_input_fmt(uint32_t fmt, u32 *val)
 {
 	switch (fmt) {
-	case DRM_FORMAT_ARGB8888:
+	case DRM_FORMAT_XRGB8888:
 		*val = 5;
 		return 0;
 
@@ -120,7 +120,6 @@ static int sun4i_frontend_drm_format_to_output_fmt(uint32_t fmt, u32 *val)
 {
 	switch (fmt) {
 	case DRM_FORMAT_XRGB8888:
-	case DRM_FORMAT_ARGB8888:
 		*val = 2;
 		return 0;
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index e0e37c2f1721..50bb16566f03 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -99,9 +99,9 @@ static void sun4i_backend_layer_atomic_update(struct drm_plane *plane,
 		sun4i_frontend_update_coord(frontend, plane);
 		sun4i_frontend_update_buffer(frontend, plane);
 		sun4i_frontend_update_formats(frontend, plane,
-					      DRM_FORMAT_ARGB8888);
+					      DRM_FORMAT_XRGB8888);
 		sun4i_backend_update_layer_frontend(backend, layer->id,
-						    DRM_FORMAT_ARGB8888);
+						    DRM_FORMAT_XRGB8888);
 		sun4i_frontend_enable(frontend);
 	} else {
 		sun4i_backend_update_layer_formats(backend, layer->id, plane);
-- 
2.19.1

Re: [PATCH v2 02/43] drm/sun4i: frontend: Replace ARGB with XRGB as supported format

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 992 bytes --]

On Fri, Nov 23, 2018 at 10:24:34AM +0100, Paul Kocialkowski wrote:
> The frontend documentation (for the A33) mentions that ARGB is supported
> as output, but with the alpha component always set to 0xff. In practice,
> this means that the alpha component cannot be preserved when going
> through the frontend. Since the information is lost, ARGB is not
> properly supported.
> 
> As a result, expose the matching format supported by the frontend (both
> for input and output) as XRGB instead of ARGB.
> 
> Since ARGB was the selected format for connecting the frontend to the
> backend, change it to XRGB to reflect this as well.
> 
> The A31 and A80 SoCs apparently have a bit to enable proper alpha,
> but this is not supported at this point (see the comment already in the
> code).
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 03/43] drm/sun4i: Add TODO comment about supporting scaling with the backend

[Paul Kocialkowski]

The backend allows integer-only scaling but can handle alpha components,
unlike the frontend. It could be useful to add support for this
eventually, so add a short TODO comment describing the situation.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 67b4bb4f5365..c3444246755b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -426,6 +426,11 @@ static bool sun4i_backend_plane_uses_frontend(struct drm_plane_state *state)
 	if (IS_ERR(backend->frontend))
 		return false;
 
+	/*
+	 * TODO: The backend alone allows 2x and 4x integer scaling, including
+	 * support for an alpha component (which the frontend doesn't support).
+	 * Use the backend directly instead of the frontend in this case.
+	 */
 	return sun4i_backend_plane_uses_scaler(state);
 }
 
-- 
2.19.1

Re: [PATCH v2 03/43] drm/sun4i: Add TODO comment about supporting scaling with the backend

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 471 bytes --]

On Fri, Nov 23, 2018 at 10:24:35AM +0100, Paul Kocialkowski wrote:
> The backend allows integer-only scaling but can handle alpha components,
> unlike the frontend. It could be useful to add support for this
> eventually, so add a short TODO comment describing the situation.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 04/43] drm/sun4i: backend: Add a helper and a list for supported formats

[Paul Kocialkowski]

In order to check whether the backend supports a specific format, an
explicit list and a related helper are introduced.

The prototype of this helper is added to the header so that it can be
called from sun4i_layer later (when introducing tiled mode support).

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 27 +++++++++++++++++++++++++++
 drivers/gpu/drm/sun4i/sun4i_backend.h |  1 +
 2 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index c3444246755b..7dd24eb03f89 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -155,6 +155,33 @@ static int sun4i_backend_drm_format_to_layer(u32 format, u32 *mode)
 	return 0;
 }
 
+static const uint32_t sun4i_backend_formats[] = {
+	DRM_FORMAT_ARGB1555,
+	DRM_FORMAT_ARGB4444,
+	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_BGRX8888,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_RGB888,
+	DRM_FORMAT_RGBA4444,
+	DRM_FORMAT_RGBA5551,
+	DRM_FORMAT_UYVY,
+	DRM_FORMAT_VYUY,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_YUYV,
+	DRM_FORMAT_YVYU,
+};
+
+bool sun4i_backend_format_is_supported(uint32_t fmt)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(sun4i_backend_formats); i++)
+		if (sun4i_backend_formats[i] == fmt)
+			return true;
+
+	return false;
+}
+
 int sun4i_backend_update_layer_coord(struct sun4i_backend *backend,
 				     int layer, struct drm_plane *plane)
 {
diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
index 339dbff1cce4..93db3af675b3 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
@@ -198,6 +198,7 @@ engine_to_sun4i_backend(struct sunxi_engine *engine)
 
 void sun4i_backend_layer_enable(struct sun4i_backend *backend,
 				int layer, bool enable);
+bool sun4i_backend_format_is_supported(uint32_t fmt);
 int sun4i_backend_update_layer_coord(struct sun4i_backend *backend,
 				     int layer, struct drm_plane *plane);
 int sun4i_backend_update_layer_formats(struct sun4i_backend *backend,
-- 
2.19.1

Re: [PATCH v2 04/43] drm/sun4i: backend: Add a helper and a list for supported formats

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 534 bytes --]

On Fri, Nov 23, 2018 at 10:24:36AM +0100, Paul Kocialkowski wrote:
> In order to check whether the backend supports a specific format, an
> explicit list and a related helper are introduced.
> 
> The prototype of this helper is added to the header so that it can be
> called from sun4i_layer later (when introducing tiled mode support).
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 05/43] drm/sun4i: frontend: Add a helper and a list for supported formats

[Paul Kocialkowski]

In order to check whether the frontend supports a specific format, an
explicit list and a related helper are introduced.

Just like in the backend, the prototype of the helper is added to the
frontend header so that it can be used later on. The helper is also
exported because it will be used outside of the frontend module.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 16 ++++++++++++++++
 drivers/gpu/drm/sun4i/sun4i_frontend.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index 3ea925584891..0f7f8c7b3402 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -128,6 +128,22 @@ static int sun4i_frontend_drm_format_to_output_fmt(uint32_t fmt, u32 *val)
 	}
 }
 
+static const uint32_t sun4i_frontend_formats[] = {
+	DRM_FORMAT_XRGB8888,
+};
+
+bool sun4i_frontend_format_is_supported(uint32_t fmt)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(sun4i_frontend_formats); i++)
+		if (sun4i_frontend_formats[i] == fmt)
+			return true;
+
+	return false;
+}
+EXPORT_SYMBOL(sun4i_frontend_format_is_supported);
+
 int sun4i_frontend_update_formats(struct sun4i_frontend *frontend,
 				  struct drm_plane *plane, uint32_t out_fmt)
 {
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.h b/drivers/gpu/drm/sun4i/sun4i_frontend.h
index 02661ce81f3e..a9cb908ced16 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.h
@@ -95,5 +95,6 @@ void sun4i_frontend_update_coord(struct sun4i_frontend *frontend,
 				 struct drm_plane *plane);
 int sun4i_frontend_update_formats(struct sun4i_frontend *frontend,
 				  struct drm_plane *plane, uint32_t out_fmt);
+bool sun4i_frontend_format_is_supported(uint32_t fmt);
 
 #endif /* _SUN4I_FRONTEND_H_ */
-- 
2.19.1

Re: [PATCH v2 05/43] drm/sun4i: frontend: Add a helper and a list for supported formats

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

On Fri, Nov 23, 2018 at 10:24:37AM +0100, Paul Kocialkowski wrote:
> In order to check whether the frontend supports a specific format, an
> explicit list and a related helper are introduced.
> 
> Just like in the backend, the prototype of the helper is added to the
> frontend header so that it can be used later on. The helper is also
> exported because it will be used outside of the frontend module.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 06/43] drm/sun4i: backend: Refine the logic behind using the frontend

[Paul Kocialkowski]

Checking that scaling is in use is not sufficient as a condition to
decide to use the frontend.

Since not all layer formats are supported by the frontend, we need to
check for that support first. Then, the frontend must only be enabled
if the backend doesn't support the format or that scaling is required.

Signed-off-by:Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 7dd24eb03f89..735fea7ead0b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -449,16 +449,32 @@ static bool sun4i_backend_plane_uses_frontend(struct drm_plane_state *state)
 {
 	struct sun4i_layer *layer = plane_to_sun4i_layer(state->plane);
 	struct sun4i_backend *backend = layer->backend;
+	uint32_t format = state->fb->format->format;
 
 	if (IS_ERR(backend->frontend))
 		return false;
 
+	if (!sun4i_frontend_format_is_supported(format))
+		return false;
+
+	if (!sun4i_backend_format_is_supported(format))
+		return true;
+
 	/*
 	 * TODO: The backend alone allows 2x and 4x integer scaling, including
 	 * support for an alpha component (which the frontend doesn't support).
-	 * Use the backend directly instead of the frontend in this case.
+	 * Use the backend directly instead of the frontend in this case, with
+	 * another test to return false.
+	 */
+
+	if (sun4i_backend_plane_uses_scaler(state))
+		return true;
+
+	/*
+	 * Here the format is supported by both the frontend and the backend
+	 * and no frontend scaling is required, so use the backend directly.
 	 */
-	return sun4i_backend_plane_uses_scaler(state);
+	return false;
 }
 
 static void sun4i_backend_atomic_begin(struct sunxi_engine *engine,
-- 
2.19.1

Re: [PATCH v2 06/43] drm/sun4i: backend: Refine the logic behind using the frontend

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 637 bytes --]

On Fri, Nov 23, 2018 at 10:24:38AM +0100, Paul Kocialkowski wrote:
> Checking that scaling is in use is not sufficient as a condition to
> decide to use the frontend.
> 
> Since not all layer formats are supported by the frontend, we need to
> check for that support first. Then, the frontend must only be enabled
> if the backend doesn't support the format or that scaling is required.
> 
> Signed-off-by:Paul Kocialkowski <paul.kocialkowski@bootlin.com>

You were missing a space to that SoB. I added it and applied, thanks!
maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 07/43] drm/sun4i: backend: Use a specific function to check if a plane is supported

[Paul Kocialkowski]

Before this patch, it is assumed that a plane is supported either
through the frontend or through the backend alone. However, the DRM
interface does not allow finely reporting our hardware capabilities
and there are cases where neither are support.

In particular, some plane formats are supported by the backend and not
the frontend, so they can only be supported without scaling.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 735fea7ead0b..52caf561da0e 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -477,6 +477,23 @@ static bool sun4i_backend_plane_uses_frontend(struct drm_plane_state *state)
 	return false;
 }
 
+static bool sun4i_backend_plane_is_supported(struct drm_plane_state *state,
+					     bool *uses_frontend)
+{
+	if (sun4i_backend_plane_uses_frontend(state)) {
+		*uses_frontend = true;
+		return true;
+	}
+
+	*uses_frontend = false;
+
+	/* Scaling is not supported without the frontend. */
+	if (sun4i_backend_plane_uses_scaler(state))
+		return false;
+
+	return true;
+}
+
 static void sun4i_backend_atomic_begin(struct sunxi_engine *engine,
 				       struct drm_crtc_state *old_state)
 {
@@ -517,14 +534,14 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
 		struct drm_framebuffer *fb = plane_state->fb;
 		struct drm_format_name_buf format_name;
 
-		if (sun4i_backend_plane_uses_frontend(plane_state)) {
+		if (!sun4i_backend_plane_is_supported(plane_state,
+						      &layer_state->uses_frontend))
+			return -EINVAL;
+
+		if (layer_state->uses_frontend) {
 			DRM_DEBUG_DRIVER("Using the frontend for plane %d\n",
 					 plane->index);
-
-			layer_state->uses_frontend = true;
 			num_frontend_planes++;
-		} else {
-			layer_state->uses_frontend = false;
 		}
 
 		DRM_DEBUG_DRIVER("Plane FB format is %s\n",
-- 
2.19.1

Re: [PATCH v2 07/43] drm/sun4i: backend: Use a specific function to check if a plane is supported

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 662 bytes --]

On Fri, Nov 23, 2018 at 10:24:39AM +0100, Paul Kocialkowski wrote:
> Before this patch, it is assumed that a plane is supported either
> through the frontend or through the backend alone. However, the DRM
> interface does not allow finely reporting our hardware capabilities
> and there are cases where neither are support.
> 
> In particular, some plane formats are supported by the backend and not
> the frontend, so they can only be supported without scaling.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 08/43] drm/fourcc: Add helper to check if a format uses a YUV colorspace

[Paul Kocialkowski]

This adds a new helper to check whether the format described by its
fourcc code uses a YUV colorspace, by returning the is_yuv entry for
the DRM info entry matching that format.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/drm_fourcc.c | 19 +++++++++++++++++++
 include/drm/drm_fourcc.h     |  1 +
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index f523948c82b1..f85650c3463a 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -413,6 +413,25 @@ int drm_format_plane_height(int height, uint32_t format, int plane)
 }
 EXPORT_SYMBOL(drm_format_plane_height);
 
+/**
+ * drm_format_is_yuv - check that the format uses a YUV colorspace
+ * @format: pixel format
+ *
+ * Returns:
+ * A boolean indicating whether the format uses a YUV colorspace.
+ */
+bool drm_format_is_yuv(uint32_t format)
+{
+	const struct drm_format_info *info;
+
+	info = drm_format_info(format);
+	if (!info)
+		return false;
+
+	return info->is_yuv;
+}
+EXPORT_SYMBOL(drm_format_is_yuv);
+
 /**
  * drm_format_info_block_width - width in pixels of block.
  * @info: pixel format info
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index bcb389f04618..569b70483505 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -157,6 +157,7 @@ int drm_format_horz_chroma_subsampling(uint32_t format);
 int drm_format_vert_chroma_subsampling(uint32_t format);
 int drm_format_plane_width(int width, uint32_t format, int plane);
 int drm_format_plane_height(int height, uint32_t format, int plane);
+bool drm_format_is_yuv(uint32_t format);
 unsigned int drm_format_info_block_width(const struct drm_format_info *info,
 					 int plane);
 unsigned int drm_format_info_block_height(const struct drm_format_info *info,
-- 
2.19.1

Re: [PATCH v2 08/43] drm/fourcc: Add helper to check if a format uses a YUV colorspace

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 486 bytes --]

On Fri, Nov 23, 2018 at 10:24:40AM +0100, Paul Kocialkowski wrote:
> This adds a new helper to check whether the format described by its
> fourcc code uses a YUV colorspace, by returning the is_yuv entry for
> the DRM info entry matching that format.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 09/43] drm/fourcc: Add format info helpers for checking YUV planes disposition

[Paul Kocialkowski]

It is often useful to check whether the DRM format info retrieved from
the DRM framebuffer matches a specific YUV planes disposition.

This introduces helpers to quickly check that a provided format info
matches a YUV format with a specific disposition, in commonly-used
terminology.

The intent of providing helpers taking the format info instead of the
fourcc alone is to avoid the overhead of iterating through all formats
when the whole format info structure is available. As a result, these
helpers are very simple so they are made inline.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 include/drm/drm_fourcc.h | 42 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 569b70483505..f43d5ba412fb 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -143,6 +143,48 @@ struct drm_format_name_buf {
 	char str[32];
 };
 
+/**
+ * drm_format_info_is_yuv_packed - check that the format info matches a YUV
+ * format with data laid in a single plane
+ * @info: format info
+ *
+ * Returns:
+ * A boolean indicating whether the format info matches a packed YUV format.
+ */
+static inline bool
+drm_format_info_is_yuv_packed(const struct drm_format_info *info)
+{
+	return info->is_yuv && info->num_planes == 1;
+}
+
+/**
+ * drm_format_info_is_yuv_semiplanar - check that the format info matches a YUV
+ * format with data laid in two planes (luminance and chrominance)
+ * @info: format info
+ *
+ * Returns:
+ * A boolean indicating whether the format info matches a semiplanar YUV format.
+ */
+static inline bool
+drm_format_info_is_yuv_semiplanar(const struct drm_format_info *info)
+{
+	return info->is_yuv && info->num_planes == 2;
+}
+
+/**
+ * drm_format_info_is_yuv_planar - check that the format info matches a YUV
+ * format with data laid in three planes (one for each YUV component)
+ * @info: format info
+ *
+ * Returns:
+ * A boolean indicating whether the format info matches a planar YUV format.
+ */
+static inline bool
+drm_format_info_is_yuv_planar(const struct drm_format_info *info)
+{
+	return info->is_yuv && info->num_planes == 3;
+}
+
 const struct drm_format_info *__drm_format_info(u32 format);
 const struct drm_format_info *drm_format_info(u32 format);
 const struct drm_format_info *
-- 
2.19.1

Re: [PATCH v2 09/43] drm/fourcc: Add format info helpers for checking YUV planes disposition

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 877 bytes --]

On Fri, Nov 23, 2018 at 10:24:41AM +0100, Paul Kocialkowski wrote:
> It is often useful to check whether the DRM format info retrieved from
> the DRM framebuffer matches a specific YUV planes disposition.
> 
> This introduces helpers to quickly check that a provided format info
> matches a YUV format with a specific disposition, in commonly-used
> terminology.
> 
> The intent of providing helpers taking the format info instead of the
> fourcc alone is to avoid the overhead of iterating through all formats
> when the whole format info structure is available. As a result, these
> helpers are very simple so they are made inline.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 10/43] drm/fourcc: Add format helpers for checking YUV planes disposition

[Paul Kocialkowski]

This introduces new format helpers that use the previously-introduced
format info helpers for checking YUV planes disposition.

Only the format fourcc is required by these helpers and the formats are
iterated from the list.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/drm_fourcc.c | 60 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_fourcc.h     |  3 ++
 2 files changed, 63 insertions(+)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index f85650c3463a..b56e773f9f63 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -432,6 +432,66 @@ bool drm_format_is_yuv(uint32_t format)
 }
 EXPORT_SYMBOL(drm_format_is_yuv);
 
+/**
+ * drm_format_is_yuv_packed - check that the format is a YUV format with data
+ * laid in a single plane
+ * @format: pixel format
+ *
+ * Returns:
+ * A boolean indicating whether the format is a packed YUV format
+ */
+bool drm_format_is_yuv_packed(uint32_t format)
+{
+	const struct drm_format_info *info;
+
+	info = drm_format_info(format);
+	if (!info)
+		return false;
+
+	return drm_format_info_is_yuv_packed(info);
+}
+EXPORT_SYMBOL(drm_format_is_yuv_packed);
+
+/**
+ * drm_format_is_yuv_semiplanar - check that the format is a YUV format with
+ * data laid in two planes (luminance and chrominance)
+ * @format: pixel format
+ *
+ * Returns:
+ * A boolean indicating whether the format is a semiplanar YUV format
+ */
+bool drm_format_is_yuv_semiplanar(uint32_t format)
+{
+	const struct drm_format_info *info;
+
+	info = drm_format_info(format);
+	if (!info)
+		return false;
+
+	return drm_format_info_is_yuv_semiplanar(info);
+}
+EXPORT_SYMBOL(drm_format_is_yuv_semiplanar);
+
+/**
+ * drm_format_is_yuv_planar - check that the format is a YUV format with data
+ * laid in three planes (one for each YUV component)
+ * @format: pixel format
+ *
+ * Returns:
+ * A boolean indicating whether the format is a planar YUV format
+ */
+bool drm_format_is_yuv_planar(uint32_t format)
+{
+	const struct drm_format_info *info;
+
+	info = drm_format_info(format);
+	if (!info)
+		return false;
+
+	return drm_format_info_is_yuv_planar(info);
+}
+EXPORT_SYMBOL(drm_format_is_yuv_planar);
+
 /**
  * drm_format_info_block_width - width in pixels of block.
  * @info: pixel format info
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index f43d5ba412fb..b652b711df1a 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -200,6 +200,9 @@ int drm_format_vert_chroma_subsampling(uint32_t format);
 int drm_format_plane_width(int width, uint32_t format, int plane);
 int drm_format_plane_height(int height, uint32_t format, int plane);
 bool drm_format_is_yuv(uint32_t format);
+bool drm_format_is_yuv_packed(uint32_t format);
+bool drm_format_is_yuv_semiplanar(uint32_t format);
+bool drm_format_is_yuv_planar(uint32_t format);
 unsigned int drm_format_info_block_width(const struct drm_format_info *info,
 					 int plane);
 unsigned int drm_format_info_block_height(const struct drm_format_info *info,
-- 
2.19.1

Re: [PATCH v2 10/43] drm/fourcc: Add format helpers for checking YUV planes disposition

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

On Fri, Nov 23, 2018 at 10:24:42AM +0100, Paul Kocialkowski wrote:
> This introduces new format helpers that use the previously-introduced
> format info helpers for checking YUV planes disposition.
> 
> Only the format fourcc is required by these helpers and the formats are
> iterated from the list.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 11/43] drm/fourcc: Add format info helpers for checking YUV sub-sampling

[Paul Kocialkowski]

Display engine drivers often need to distinguish between different types of
YUV sub-sampling. This introduces helpers to check for common sub-sampling
ratios in their commonly-used denomination from the DRM format info.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 include/drm/drm_fourcc.h | 75 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index b652b711df1a..d170b7b323f7 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -185,6 +185,81 @@ drm_format_info_is_yuv_planar(const struct drm_format_info *info)
 	return info->is_yuv && info->num_planes == 3;
 }
 
+/**
+ * drm_format_info_is_yuv_sampling_410 - check that the format info matches a
+ * YUV format with 4:1:0 sub-sampling
+ * @info: format info
+ *
+ * Returns:
+ * A boolean indicating whether the format info matches a YUV format with 4:1:0
+ * sub-sampling.
+ */
+static inline bool
+drm_format_info_is_yuv_sampling_410(const struct drm_format_info *info)
+{
+	return info->is_yuv && info->hsub == 4 && info->vsub == 4;
+}
+
+/**
+ * drm_format_info_is_yuv_sampling_411 - check that the format info matches a
+ * YUV format with 4:1:1 sub-sampling
+ * @info: format info
+ *
+ * Returns:
+ * A boolean indicating whether the format info matches a YUV format with 4:1:1
+ * sub-sampling.
+ */
+static inline bool
+drm_format_info_is_yuv_sampling_411(const struct drm_format_info *info)
+{
+	return info->is_yuv && info->hsub == 4 && info->vsub == 1;
+}
+
+/**
+ * drm_format_info_is_yuv_sampling_420 - check that the format info matches a
+ * YUV format with 4:2:0 sub-sampling
+ * @info: format info
+ *
+ * Returns:
+ * A boolean indicating whether the format info matches a YUV format with 4:2:0
+ * sub-sampling.
+ */
+static inline bool
+drm_format_info_is_yuv_sampling_420(const struct drm_format_info *info)
+{
+	return info->is_yuv && info->hsub == 2 && info->vsub == 2;
+}
+
+/**
+ * drm_format_info_is_yuv_sampling_422 - check that the format info matches a
+ * YUV format with 4:2:2 sub-sampling
+ * @info: format info
+ *
+ * Returns:
+ * A boolean indicating whether the format info matches a YUV format with 4:2:2
+ * sub-sampling.
+ */
+static inline bool
+drm_format_info_is_yuv_sampling_422(const struct drm_format_info *info)
+{
+	return info->is_yuv && info->hsub == 2 && info->vsub == 1;
+}
+
+/**
+ * drm_format_info_is_yuv_sampling_444 - check that the format info matches a
+ * YUV format with 4:4:4 sub-sampling
+ * @info: format info
+ *
+ * Returns:
+ * A boolean indicating whether the format info matches a YUV format with 4:4:4
+ * sub-sampling.
+ */
+static inline bool
+drm_format_info_is_yuv_sampling_444(const struct drm_format_info *info)
+{
+	return info->is_yuv && info->hsub == 1 && info->vsub == 1;
+}
+
 const struct drm_format_info *__drm_format_info(u32 format);
 const struct drm_format_info *drm_format_info(u32 format);
 const struct drm_format_info *
-- 
2.19.1

Re: [PATCH v2 11/43] drm/fourcc: Add format info helpers for checking YUV sub-sampling

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 528 bytes --]

On Fri, Nov 23, 2018 at 10:24:43AM +0100, Paul Kocialkowski wrote:
> Display engine drivers often need to distinguish between different types of
> YUV sub-sampling. This introduces helpers to check for common sub-sampling
> ratios in their commonly-used denomination from the DRM format info.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 12/43] drm/fourcc: Add format helpers for checking YUV sub-sampling

[Paul Kocialkowski]

This introduces new format helpers that use the previously-introduced
format info helpers for checking YUV sub-sampling.

Only the format fourcc is required by these helpers and the formats are
iterated from the list.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/drm_fourcc.c | 105 +++++++++++++++++++++++++++++++++++
 include/drm/drm_fourcc.h     |   5 ++
 2 files changed, 110 insertions(+)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index b56e773f9f63..6d395c586ad5 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -492,6 +492,111 @@ bool drm_format_is_yuv_planar(uint32_t format)
 }
 EXPORT_SYMBOL(drm_format_is_yuv_planar);
 
+/**
+ * drm_format_is_yuv_sampling_410 - check that the format is a YUV format with
+ * 4:1:0 sub-sampling
+ * @format: pixel format
+ *
+ * Returns:
+ * A boolean indicating whether the format is a YUV format with 4:1:0
+ * sub-sampling.
+ */
+bool drm_format_is_yuv_sampling_410(uint32_t format)
+{
+	const struct drm_format_info *info;
+
+	info = drm_format_info(format);
+	if (!info)
+		return false;
+
+	return drm_format_info_is_yuv_sampling_410(info);
+}
+EXPORT_SYMBOL(drm_format_is_yuv_sampling_410);
+
+/**
+ * drm_format_is_yuv_sampling_411 - check that the format is a YUV format with
+ * 4:1:1 sub-sampling
+ * @format: pixel format
+ *
+ * Returns:
+ * A boolean indicating whether the format is a YUV format with 4:1:1
+ * sub-sampling.
+ */
+bool drm_format_is_yuv_sampling_411(uint32_t format)
+{
+	const struct drm_format_info *info;
+
+	info = drm_format_info(format);
+	if (!info)
+		return false;
+
+	return drm_format_info_is_yuv_sampling_411(info);
+}
+EXPORT_SYMBOL(drm_format_is_yuv_sampling_411);
+
+/**
+ * drm_format_is_yuv_sampling_420 - check that the format is a YUV format with
+ * 4:2:0 sub-sampling
+ * @format: pixel format
+ *
+ * Returns:
+ * A boolean indicating whether the format is a YUV format with 4:2:0
+ * sub-sampling.
+ */
+bool drm_format_is_yuv_sampling_420(uint32_t format)
+{
+	const struct drm_format_info *info;
+
+	info = drm_format_info(format);
+	if (!info)
+		return false;
+
+	return drm_format_info_is_yuv_sampling_420(info);
+}
+EXPORT_SYMBOL(drm_format_is_yuv_sampling_420);
+
+/**
+ * drm_format_is_yuv_sampling_422 - check that the format is a YUV format with
+ * 4:2:2 sub-sampling
+ * @format: pixel format
+ *
+ * Returns:
+ * A boolean indicating whether the format is a YUV format with 4:2:2
+ * sub-sampling.
+ */
+bool drm_format_is_yuv_sampling_422(uint32_t format)
+{
+	const struct drm_format_info *info;
+
+	info = drm_format_info(format);
+	if (!info)
+		return false;
+
+	return drm_format_info_is_yuv_sampling_422(info);
+}
+EXPORT_SYMBOL(drm_format_is_yuv_sampling_422);
+
+/**
+ * drm_format_is_yuv_sampling_444 - check that the format is a YUV format with
+ * 4:4:4 sub-sampling
+ * @format: pixel format
+ *
+ * Returns:
+ * A boolean indicating whether the format is a YUV format with 4:4:4
+ * sub-sampling.
+ */
+bool drm_format_is_yuv_sampling_444(uint32_t format)
+{
+	const struct drm_format_info *info;
+
+	info = drm_format_info(format);
+	if (!info)
+		return false;
+
+	return drm_format_info_is_yuv_sampling_444(info);
+}
+EXPORT_SYMBOL(drm_format_is_yuv_sampling_444);
+
 /**
  * drm_format_info_block_width - width in pixels of block.
  * @info: pixel format info
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index d170b7b323f7..cf082d8b6ad4 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -278,6 +278,11 @@ bool drm_format_is_yuv(uint32_t format);
 bool drm_format_is_yuv_packed(uint32_t format);
 bool drm_format_is_yuv_semiplanar(uint32_t format);
 bool drm_format_is_yuv_planar(uint32_t format);
+bool drm_format_is_yuv_sampling_410(uint32_t format);
+bool drm_format_is_yuv_sampling_411(uint32_t format);
+bool drm_format_is_yuv_sampling_420(uint32_t format);
+bool drm_format_is_yuv_sampling_422(uint32_t format);
+bool drm_format_is_yuv_sampling_444(uint32_t format);
 unsigned int drm_format_info_block_width(const struct drm_format_info *info,
 					 int plane);
 unsigned int drm_format_info_block_height(const struct drm_format_info *info,
-- 
2.19.1

Re: [PATCH v2 12/43] drm/fourcc: Add format helpers for checking YUV sub-sampling

[Ayan Halder]

On Fri, Nov 23, 2018 at 10:24:44AM +0100, Paul Kocialkowski wrote:

Hi Paul,

> This introduces new format helpers that use the previously-introduced
> format info helpers for checking YUV sub-sampling.
> 
> Only the format fourcc is required by these helpers and the formats are
> iterated from the list.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  drivers/gpu/drm/drm_fourcc.c | 105 +++++++++++++++++++++++++++++++++++
>  include/drm/drm_fourcc.h     |   5 ++
>  2 files changed, 110 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index b56e773f9f63..6d395c586ad5 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -492,6 +492,111 @@ bool drm_format_is_yuv_planar(uint32_t format)
>  }
>  EXPORT_SYMBOL(drm_format_is_yuv_planar);
>  
> +/**
> + * drm_format_is_yuv_sampling_410 - check that the format is a YUV format with
> + * 4:1:0 sub-sampling
> + * @format: pixel format
> + *
> + * Returns:
> + * A boolean indicating whether the format is a YUV format with 4:1:0
> + * sub-sampling.
> + */
> +bool drm_format_is_yuv_sampling_410(uint32_t format)
> +{
> +	const struct drm_format_info *info;
> +
> +	info = drm_format_info(format);
> +	if (!info)
> +		return false;
> +
> +	return drm_format_info_is_yuv_sampling_410(info);
> +}
> +EXPORT_SYMBOL(drm_format_is_yuv_sampling_410);
> +
> +/**
> + * drm_format_is_yuv_sampling_411 - check that the format is a YUV format with
> + * 4:1:1 sub-sampling
> + * @format: pixel format
> + *
> + * Returns:
> + * A boolean indicating whether the format is a YUV format with 4:1:1
> + * sub-sampling.
> + */
> +bool drm_format_is_yuv_sampling_411(uint32_t format)
> +{
> +	const struct drm_format_info *info;
> +
> +	info = drm_format_info(format);
> +	if (!info)
> +		return false;
> +
> +	return drm_format_info_is_yuv_sampling_411(info);
> +}
> +EXPORT_SYMBOL(drm_format_is_yuv_sampling_411);
> +
> +/**
> + * drm_format_is_yuv_sampling_420 - check that the format is a YUV format with
> + * 4:2:0 sub-sampling
> + * @format: pixel format
> + *
> + * Returns:
> + * A boolean indicating whether the format is a YUV format with 4:2:0
> + * sub-sampling.
> + */
> +bool drm_format_is_yuv_sampling_420(uint32_t format)
> +{
> +	const struct drm_format_info *info;
> +
> +	info = drm_format_info(format);
> +	if (!info)
> +		return false;
> +
> +	return drm_format_info_is_yuv_sampling_420(info);
> +}
> +EXPORT_SYMBOL(drm_format_is_yuv_sampling_420);
> +
> +/**
> + * drm_format_is_yuv_sampling_422 - check that the format is a YUV format with
> + * 4:2:2 sub-sampling
> + * @format: pixel format
> + *
> + * Returns:
> + * A boolean indicating whether the format is a YUV format with 4:2:2
> + * sub-sampling.
> + */
> +bool drm_format_is_yuv_sampling_422(uint32_t format)
> +{
> +	const struct drm_format_info *info;
> +
> +	info = drm_format_info(format);
> +	if (!info)
> +		return false;
> +
> +	return drm_format_info_is_yuv_sampling_422(info);
> +}
> +EXPORT_SYMBOL(drm_format_is_yuv_sampling_422);
> +
> +/**
> + * drm_format_is_yuv_sampling_444 - check that the format is a YUV format with
> + * 4:4:4 sub-sampling
> + * @format: pixel format
> + *
> + * Returns:
> + * A boolean indicating whether the format is a YUV format with 4:4:4
> + * sub-sampling.
> + */
> +bool drm_format_is_yuv_sampling_444(uint32_t format)
> +{
> +	const struct drm_format_info *info;
> +
> +	info = drm_format_info(format);
> +	if (!info)
> +		return false;
> +
> +	return drm_format_info_is_yuv_sampling_444(info);
> +}
> +EXPORT_SYMBOL(drm_format_is_yuv_sampling_444);
> +
>  /**
>   * drm_format_info_block_width - width in pixels of block.
>   * @info: pixel format info
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index d170b7b323f7..cf082d8b6ad4 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -278,6 +278,11 @@ bool drm_format_is_yuv(uint32_t format);
>  bool drm_format_is_yuv_packed(uint32_t format);
>  bool drm_format_is_yuv_semiplanar(uint32_t format);
>  bool drm_format_is_yuv_planar(uint32_t format);
> +bool drm_format_is_yuv_sampling_410(uint32_t format);
> +bool drm_format_is_yuv_sampling_411(uint32_t format);
> +bool drm_format_is_yuv_sampling_420(uint32_t format);
> +bool drm_format_is_yuv_sampling_422(uint32_t format);
> +bool drm_format_is_yuv_sampling_444(uint32_t format);
>  unsigned int drm_format_info_block_width(const struct drm_format_info *info,
>  					 int plane);
>  unsigned int drm_format_info_block_height(const struct drm_format_info *info,
> -- 
> 2.19.1
> 
From patches [PATCH v2 08/43] till [PATCH v2 12/43], looks good to me.

How about clubbing all the drm_format_helper functions together in a
separate file (call it drm_fourcc_helper.c)? This way, we could
keep the __drm_format_info[] only drm_fourcc.c.

> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 12/43] drm/fourcc: Add format helpers for checking YUV sub-sampling

[Ville Syrjälä]

On Fri, Nov 23, 2018 at 04:55:33PM +0000, Ayan Halder wrote:
> On Fri, Nov 23, 2018 at 10:24:44AM +0100, Paul Kocialkowski wrote:
> 
> Hi Paul,
> 
> > This introduces new format helpers that use the previously-introduced
> > format info helpers for checking YUV sub-sampling.
> > 
> > Only the format fourcc is required by these helpers and the formats are
> > iterated from the list.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  drivers/gpu/drm/drm_fourcc.c | 105 +++++++++++++++++++++++++++++++++++
> >  include/drm/drm_fourcc.h     |   5 ++
> >  2 files changed, 110 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > index b56e773f9f63..6d395c586ad5 100644
> > --- a/drivers/gpu/drm/drm_fourcc.c
> > +++ b/drivers/gpu/drm/drm_fourcc.c
> > @@ -492,6 +492,111 @@ bool drm_format_is_yuv_planar(uint32_t format)
> >  }
> >  EXPORT_SYMBOL(drm_format_is_yuv_planar);
> >  
> > +/**
> > + * drm_format_is_yuv_sampling_410 - check that the format is a YUV format with
> > + * 4:1:0 sub-sampling
> > + * @format: pixel format
> > + *
> > + * Returns:
> > + * A boolean indicating whether the format is a YUV format with 4:1:0
> > + * sub-sampling.
> > + */
> > +bool drm_format_is_yuv_sampling_410(uint32_t format)
> > +{
> > +	const struct drm_format_info *info;
> > +
> > +	info = drm_format_info(format);
> > +	if (!info)
> > +		return false;

Looks like you're doing a lot of drm_format_info(fb->format->format),
which doesn't make sense. Not to mention that each lookup is a linear
search so the costs may start to add up.

Unless there's a really good reason I'd rather we try to remove all
the current helpers that do these lookups, and not add any more.

> > +
> > +	return drm_format_info_is_yuv_sampling_410(info);
> > +}
> > +EXPORT_SYMBOL(drm_format_is_yuv_sampling_410);
> > +
> > +/**
> > + * drm_format_is_yuv_sampling_411 - check that the format is a YUV format with
> > + * 4:1:1 sub-sampling
> > + * @format: pixel format
> > + *
> > + * Returns:
> > + * A boolean indicating whether the format is a YUV format with 4:1:1
> > + * sub-sampling.
> > + */
> > +bool drm_format_is_yuv_sampling_411(uint32_t format)
> > +{
> > +	const struct drm_format_info *info;
> > +
> > +	info = drm_format_info(format);
> > +	if (!info)
> > +		return false;
> > +
> > +	return drm_format_info_is_yuv_sampling_411(info);
> > +}
> > +EXPORT_SYMBOL(drm_format_is_yuv_sampling_411);
> > +
> > +/**
> > + * drm_format_is_yuv_sampling_420 - check that the format is a YUV format with
> > + * 4:2:0 sub-sampling
> > + * @format: pixel format
> > + *
> > + * Returns:
> > + * A boolean indicating whether the format is a YUV format with 4:2:0
> > + * sub-sampling.
> > + */
> > +bool drm_format_is_yuv_sampling_420(uint32_t format)
> > +{
> > +	const struct drm_format_info *info;
> > +
> > +	info = drm_format_info(format);
> > +	if (!info)
> > +		return false;
> > +
> > +	return drm_format_info_is_yuv_sampling_420(info);
> > +}
> > +EXPORT_SYMBOL(drm_format_is_yuv_sampling_420);
> > +
> > +/**
> > + * drm_format_is_yuv_sampling_422 - check that the format is a YUV format with
> > + * 4:2:2 sub-sampling
> > + * @format: pixel format
> > + *
> > + * Returns:
> > + * A boolean indicating whether the format is a YUV format with 4:2:2
> > + * sub-sampling.
> > + */
> > +bool drm_format_is_yuv_sampling_422(uint32_t format)
> > +{
> > +	const struct drm_format_info *info;
> > +
> > +	info = drm_format_info(format);
> > +	if (!info)
> > +		return false;
> > +
> > +	return drm_format_info_is_yuv_sampling_422(info);
> > +}
> > +EXPORT_SYMBOL(drm_format_is_yuv_sampling_422);
> > +
> > +/**
> > + * drm_format_is_yuv_sampling_444 - check that the format is a YUV format with
> > + * 4:4:4 sub-sampling
> > + * @format: pixel format
> > + *
> > + * Returns:
> > + * A boolean indicating whether the format is a YUV format with 4:4:4
> > + * sub-sampling.
> > + */
> > +bool drm_format_is_yuv_sampling_444(uint32_t format)
> > +{
> > +	const struct drm_format_info *info;
> > +
> > +	info = drm_format_info(format);
> > +	if (!info)
> > +		return false;
> > +
> > +	return drm_format_info_is_yuv_sampling_444(info);
> > +}
> > +EXPORT_SYMBOL(drm_format_is_yuv_sampling_444);
> > +
> >  /**
> >   * drm_format_info_block_width - width in pixels of block.
> >   * @info: pixel format info
> > diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> > index d170b7b323f7..cf082d8b6ad4 100644
> > --- a/include/drm/drm_fourcc.h
> > +++ b/include/drm/drm_fourcc.h
> > @@ -278,6 +278,11 @@ bool drm_format_is_yuv(uint32_t format);
> >  bool drm_format_is_yuv_packed(uint32_t format);
> >  bool drm_format_is_yuv_semiplanar(uint32_t format);
> >  bool drm_format_is_yuv_planar(uint32_t format);
> > +bool drm_format_is_yuv_sampling_410(uint32_t format);
> > +bool drm_format_is_yuv_sampling_411(uint32_t format);
> > +bool drm_format_is_yuv_sampling_420(uint32_t format);
> > +bool drm_format_is_yuv_sampling_422(uint32_t format);
> > +bool drm_format_is_yuv_sampling_444(uint32_t format);
> >  unsigned int drm_format_info_block_width(const struct drm_format_info *info,
> >  					 int plane);
> >  unsigned int drm_format_info_block_height(const struct drm_format_info *info,
> > -- 
> > 2.19.1
> > 
> From patches [PATCH v2 08/43] till [PATCH v2 12/43], looks good to me.
> 
> How about clubbing all the drm_format_helper functions together in a
> separate file (call it drm_fourcc_helper.c)? This way, we could
> keep the __drm_format_info[] only drm_fourcc.c.
> 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel

Re: [PATCH v2 12/43] drm/fourcc: Add format helpers for checking YUV sub-sampling

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 7089 bytes --]

Hi,

On Fri, 2018-11-23 at 19:23 +0200, Ville Syrjälä wrote:
> On Fri, Nov 23, 2018 at 04:55:33PM +0000, Ayan Halder wrote:
> > On Fri, Nov 23, 2018 at 10:24:44AM +0100, Paul Kocialkowski wrote:
> > 
> > Hi Paul,
> > 
> > > This introduces new format helpers that use the previously-introduced
> > > format info helpers for checking YUV sub-sampling.
> > > 
> > > Only the format fourcc is required by these helpers and the formats are
> > > iterated from the list.
> > > 
> > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > ---
> > >  drivers/gpu/drm/drm_fourcc.c | 105 +++++++++++++++++++++++++++++++++++
> > >  include/drm/drm_fourcc.h     |   5 ++
> > >  2 files changed, 110 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > > index b56e773f9f63..6d395c586ad5 100644
> > > --- a/drivers/gpu/drm/drm_fourcc.c
> > > +++ b/drivers/gpu/drm/drm_fourcc.c
> > > @@ -492,6 +492,111 @@ bool drm_format_is_yuv_planar(uint32_t format)
> > >  }
> > >  EXPORT_SYMBOL(drm_format_is_yuv_planar);
> > >  
> > > +/**
> > > + * drm_format_is_yuv_sampling_410 - check that the format is a YUV format with
> > > + * 4:1:0 sub-sampling
> > > + * @format: pixel format
> > > + *
> > > + * Returns:
> > > + * A boolean indicating whether the format is a YUV format with 4:1:0
> > > + * sub-sampling.
> > > + */
> > > +bool drm_format_is_yuv_sampling_410(uint32_t format)
> > > +{
> > > +	const struct drm_format_info *info;
> > > +
> > > +	info = drm_format_info(format);
> > > +	if (!info)
> > > +		return false;
> 
> Looks like you're doing a lot of drm_format_info(fb->format->format),
> which doesn't make sense. Not to mention that each lookup is a linear
> search so the costs may start to add up.
> 
> Unless there's a really good reason I'd rather we try to remove all
> the current helpers that do these lookups, and not add any more.

I totally agree that we should be passing the drm_format_info structure
onwards to all the driver helpers so we don't have to do the lookup each
time.

I thought it was still somewhat acceptable to have lookup fashions of
these format helpers, but I am totally in favor of getting rid of them
(and other lookup helpers eventually).

I'll remove the lookup YUV format helpers and convert this series to
passing the full drm_format_info everywhere needed.

Cheers,

Paul

> > > +
> > > +	return drm_format_info_is_yuv_sampling_410(info);
> > > +}
> > > +EXPORT_SYMBOL(drm_format_is_yuv_sampling_410);
> > > +
> > > +/**
> > > + * drm_format_is_yuv_sampling_411 - check that the format is a YUV format with
> > > + * 4:1:1 sub-sampling
> > > + * @format: pixel format
> > > + *
> > > + * Returns:
> > > + * A boolean indicating whether the format is a YUV format with 4:1:1
> > > + * sub-sampling.
> > > + */
> > > +bool drm_format_is_yuv_sampling_411(uint32_t format)
> > > +{
> > > +	const struct drm_format_info *info;
> > > +
> > > +	info = drm_format_info(format);
> > > +	if (!info)
> > > +		return false;
> > > +
> > > +	return drm_format_info_is_yuv_sampling_411(info);
> > > +}
> > > +EXPORT_SYMBOL(drm_format_is_yuv_sampling_411);
> > > +
> > > +/**
> > > + * drm_format_is_yuv_sampling_420 - check that the format is a YUV format with
> > > + * 4:2:0 sub-sampling
> > > + * @format: pixel format
> > > + *
> > > + * Returns:
> > > + * A boolean indicating whether the format is a YUV format with 4:2:0
> > > + * sub-sampling.
> > > + */
> > > +bool drm_format_is_yuv_sampling_420(uint32_t format)
> > > +{
> > > +	const struct drm_format_info *info;
> > > +
> > > +	info = drm_format_info(format);
> > > +	if (!info)
> > > +		return false;
> > > +
> > > +	return drm_format_info_is_yuv_sampling_420(info);
> > > +}
> > > +EXPORT_SYMBOL(drm_format_is_yuv_sampling_420);
> > > +
> > > +/**
> > > + * drm_format_is_yuv_sampling_422 - check that the format is a YUV format with
> > > + * 4:2:2 sub-sampling
> > > + * @format: pixel format
> > > + *
> > > + * Returns:
> > > + * A boolean indicating whether the format is a YUV format with 4:2:2
> > > + * sub-sampling.
> > > + */
> > > +bool drm_format_is_yuv_sampling_422(uint32_t format)
> > > +{
> > > +	const struct drm_format_info *info;
> > > +
> > > +	info = drm_format_info(format);
> > > +	if (!info)
> > > +		return false;
> > > +
> > > +	return drm_format_info_is_yuv_sampling_422(info);
> > > +}
> > > +EXPORT_SYMBOL(drm_format_is_yuv_sampling_422);
> > > +
> > > +/**
> > > + * drm_format_is_yuv_sampling_444 - check that the format is a YUV format with
> > > + * 4:4:4 sub-sampling
> > > + * @format: pixel format
> > > + *
> > > + * Returns:
> > > + * A boolean indicating whether the format is a YUV format with 4:4:4
> > > + * sub-sampling.
> > > + */
> > > +bool drm_format_is_yuv_sampling_444(uint32_t format)
> > > +{
> > > +	const struct drm_format_info *info;
> > > +
> > > +	info = drm_format_info(format);
> > > +	if (!info)
> > > +		return false;
> > > +
> > > +	return drm_format_info_is_yuv_sampling_444(info);
> > > +}
> > > +EXPORT_SYMBOL(drm_format_is_yuv_sampling_444);
> > > +
> > >  /**
> > >   * drm_format_info_block_width - width in pixels of block.
> > >   * @info: pixel format info
> > > diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> > > index d170b7b323f7..cf082d8b6ad4 100644
> > > --- a/include/drm/drm_fourcc.h
> > > +++ b/include/drm/drm_fourcc.h
> > > @@ -278,6 +278,11 @@ bool drm_format_is_yuv(uint32_t format);
> > >  bool drm_format_is_yuv_packed(uint32_t format);
> > >  bool drm_format_is_yuv_semiplanar(uint32_t format);
> > >  bool drm_format_is_yuv_planar(uint32_t format);
> > > +bool drm_format_is_yuv_sampling_410(uint32_t format);
> > > +bool drm_format_is_yuv_sampling_411(uint32_t format);
> > > +bool drm_format_is_yuv_sampling_420(uint32_t format);
> > > +bool drm_format_is_yuv_sampling_422(uint32_t format);
> > > +bool drm_format_is_yuv_sampling_444(uint32_t format);
> > >  unsigned int drm_format_info_block_width(const struct drm_format_info *info,
> > >  					 int plane);
> > >  unsigned int drm_format_info_block_height(const struct drm_format_info *info,
> > > -- 
> > > 2.19.1
> > > 
> > From patches [PATCH v2 08/43] till [PATCH v2 12/43], looks good to me.
> > 
> > How about clubbing all the drm_format_helper functions together in a
> > separate file (call it drm_fourcc_helper.c)? This way, we could
> > keep the __drm_format_info[] only drm_fourcc.c.
> > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 13/43] drm/sun4i: backend: Use explicit fourcc helpers for packed YUV422 check

[Paul Kocialkowski]

Checking for the number of planes is not sufficient to en ensure that
the format is a packed YUV422.

Use explicit fourcc helpers for the check instead.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 52caf561da0e..30d7bc76c0af 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -238,7 +238,8 @@ static int sun4i_backend_update_yuv_format(struct sun4i_backend *backend,
 			   SUN4I_BACKEND_ATTCTL_REG0_LAY_YUVEN);
 
 	/* TODO: Add support for the multi-planar YUV formats */
-	if (format->num_planes == 1)
+	if (drm_format_info_is_yuv_packed(format) &&
+	    drm_format_info_is_yuv_sampling_422(format))
 		val |= SUN4I_BACKEND_IYUVCTL_FBFMT_PACKED_YUV422;
 	else
 		DRM_DEBUG_DRIVER("Unsupported YUV format (0x%x)\n", fmt);
-- 
2.19.1

Re: [PATCH v2 13/43] drm/sun4i: backend: Use explicit fourcc helpers for packed YUV422 check

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 461 bytes --]

On Fri, Nov 23, 2018 at 10:24:45AM +0100, Paul Kocialkowski wrote:
> Checking for the number of planes is not sufficient to en ensure that
> the format is a packed YUV422.
> 
> Use explicit fourcc helpers for the check instead.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 14/43] drm/sun4i: backend: Avoid counting YUV planes that use the frontend

[Paul Kocialkowski]

Our hardware has a limited number of YUV planes (usually 1) that can be
supported using the backend only. However, YUV planes can also be
supported by the frontend and must then not be counted when checking for
that limitation.

Only count the YUV plane when the frontend is not used.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 30d7bc76c0af..a39d8b75d76a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -543,6 +543,11 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
 			DRM_DEBUG_DRIVER("Using the frontend for plane %d\n",
 					 plane->index);
 			num_frontend_planes++;
+		} else {
+			if (fb->format->is_yuv) {
+				DRM_DEBUG_DRIVER("Plane FB format is YUV\n");
+				num_yuv_planes++;
+			}
 		}
 
 		DRM_DEBUG_DRIVER("Plane FB format is %s\n",
@@ -551,11 +556,6 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
 		if (fb->format->has_alpha || (plane_state->alpha != DRM_BLEND_ALPHA_OPAQUE))
 			num_alpha_planes++;
 
-		if (fb->format->is_yuv) {
-			DRM_DEBUG_DRIVER("Plane FB format is YUV\n");
-			num_yuv_planes++;
-		}
-
 		DRM_DEBUG_DRIVER("Plane zpos is %d\n",
 				 plane_state->normalized_zpos);
 
-- 
2.19.1

Re: [PATCH v2 14/43] drm/sun4i: backend: Avoid counting YUV planes that use the frontend

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 562 bytes --]

On Fri, Nov 23, 2018 at 10:24:46AM +0100, Paul Kocialkowski wrote:
> Our hardware has a limited number of YUV planes (usually 1) that can be
> supported using the backend only. However, YUV planes can also be
> supported by the frontend and must then not be counted when checking for
> that limitation.
> 
> Only count the YUV plane when the frontend is not used.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 15/43] drm/sun4i: Rename sun4i_backend_layer_formats to sun4i_layer_formats

[Paul Kocialkowski]

Since more formats can be supported by the frontend, rename the
variable listing the layer formats to avoid suggesting that the backend
itself supports all the listed formats.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_layer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index 50bb16566f03..c3368b17e5f1 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -127,7 +127,7 @@ static const struct drm_plane_funcs sun4i_backend_layer_funcs = {
 	.update_plane		= drm_atomic_helper_update_plane,
 };
 
-static const uint32_t sun4i_backend_layer_formats[] = {
+static const uint32_t sun4i_layer_formats[] = {
 	DRM_FORMAT_ARGB8888,
 	DRM_FORMAT_ARGB4444,
 	DRM_FORMAT_ARGB1555,
@@ -156,8 +156,8 @@ static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
 	/* possible crtcs are set later */
 	ret = drm_universal_plane_init(drm, &layer->plane, 0,
 				       &sun4i_backend_layer_funcs,
-				       sun4i_backend_layer_formats,
-				       ARRAY_SIZE(sun4i_backend_layer_formats),
+				       sun4i_layer_formats,
+				       ARRAY_SIZE(sun4i_layer_formats),
 				       NULL, type, NULL);
 	if (ret) {
 		dev_err(drm->dev, "Couldn't initialize layer\n");
-- 
2.19.1

Re: [PATCH v2 15/43] drm/sun4i: Rename sun4i_backend_layer_formats to sun4i_layer_formats

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 444 bytes --]

On Fri, Nov 23, 2018 at 10:24:47AM +0100, Paul Kocialkowski wrote:
> Since more formats can be supported by the frontend, rename the
> variable listing the layer formats to avoid suggesting that the backend
> itself supports all the listed formats.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 16/43] drm/sun4i: frontend: Move CSC bypass setup to format update routine

[Paul Kocialkowski]

In order to support YUV to RGB conversion with the frontend (which is
generally used for connecting with the backend), the CSC block must not
be bypassed.

As a result, the bit to enable CSC bypass is moved from the runtime
resume routine to the format update routine, so that it can disabled
when introducing support for YUV formats later.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index 0f7f8c7b3402..f54b1c4a9264 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -177,6 +177,10 @@ int sun4i_frontend_update_formats(struct sun4i_frontend *frontend,
 	regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_VERTPHASE1_REG, 0x400);
 	regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_VERTPHASE1_REG, 0x400);
 
+	regmap_update_bits(frontend->regs, SUN4I_FRONTEND_BYPASS_REG,
+			   SUN4I_FRONTEND_BYPASS_CSC_EN,
+			   SUN4I_FRONTEND_BYPASS_CSC_EN);
+
 	regmap_write(frontend->regs, SUN4I_FRONTEND_INPUT_FMT_REG,
 		     SUN4I_FRONTEND_INPUT_FMT_DATA_MOD(1) |
 		     SUN4I_FRONTEND_INPUT_FMT_DATA_FMT(in_fmt_val) |
@@ -354,10 +358,6 @@ static int sun4i_frontend_runtime_resume(struct device *dev)
 			   SUN4I_FRONTEND_EN_EN,
 			   SUN4I_FRONTEND_EN_EN);
 
-	regmap_update_bits(frontend->regs, SUN4I_FRONTEND_BYPASS_REG,
-			   SUN4I_FRONTEND_BYPASS_CSC_EN,
-			   SUN4I_FRONTEND_BYPASS_CSC_EN);
-
 	sun4i_frontend_scaler_init(frontend);
 
 	return 0;
-- 
2.19.1

Re: [PATCH v2 16/43] drm/sun4i: frontend: Move CSC bypass setup to format update routine

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 621 bytes --]

On Fri, Nov 23, 2018 at 10:24:48AM +0100, Paul Kocialkowski wrote:
> In order to support YUV to RGB conversion with the frontend (which is
> generally used for connecting with the backend), the CSC block must not
> be bypassed.
> 
> As a result, the bit to enable CSC bypass is moved from the runtime
> resume routine to the format update routine, so that it can disabled
> when introducing support for YUV formats later.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 17/43] drm/sun4i: frontend: Add helpers for input data mode and pixel sequence

[Paul Kocialkowski]

This introduces new helpers for retrieving the input data mode and pixel
sequence register field values based on the DRM format instead of
hardcoding these. This makes it easier to add support for more formats.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 46 +++++++++++++++++++++++---
 1 file changed, 41 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index f54b1c4a9264..bb5977a466f3 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -116,6 +116,30 @@ static int sun4i_frontend_drm_format_to_input_fmt(uint32_t fmt, u32 *val)
 	}
 }
 
+static int sun4i_frontend_drm_format_to_input_mode(uint32_t fmt, u32 *val)
+{
+	switch (fmt) {
+	case DRM_FORMAT_XRGB8888:
+		*val = 1;
+		return 0;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int sun4i_frontend_drm_format_to_input_sequence(uint32_t fmt, u32 *val)
+{
+	switch (fmt) {
+	case DRM_FORMAT_XRGB8888:
+		*val = 1;
+		return 0;
+
+	default:
+		return -EINVAL;
+	}
+}
+
 static int sun4i_frontend_drm_format_to_output_fmt(uint32_t fmt, u32 *val)
 {
 	switch (fmt) {
@@ -149,17 +173,29 @@ int sun4i_frontend_update_formats(struct sun4i_frontend *frontend,
 {
 	struct drm_plane_state *state = plane->state;
 	struct drm_framebuffer *fb = state->fb;
+	uint32_t format = fb->format->format;
 	u32 out_fmt_val;
-	u32 in_fmt_val;
+	u32 in_fmt_val, in_mod_val, in_ps_val;
 	int ret;
 
-	ret = sun4i_frontend_drm_format_to_input_fmt(fb->format->format,
-						     &in_fmt_val);
+	ret = sun4i_frontend_drm_format_to_input_fmt(format, &in_fmt_val);
 	if (ret) {
 		DRM_DEBUG_DRIVER("Invalid input format\n");
 		return ret;
 	}
 
+	ret = sun4i_frontend_drm_format_to_input_mode(format, &in_mod_val);
+	if (ret) {
+		DRM_DEBUG_DRIVER("Invalid input mode\n");
+		return ret;
+	}
+
+	ret = sun4i_frontend_drm_format_to_input_sequence(format, &in_ps_val);
+	if (ret) {
+		DRM_DEBUG_DRIVER("Invalid pixel sequence\n");
+		return ret;
+	}
+
 	ret = sun4i_frontend_drm_format_to_output_fmt(out_fmt, &out_fmt_val);
 	if (ret) {
 		DRM_DEBUG_DRIVER("Invalid output format\n");
@@ -182,9 +218,9 @@ int sun4i_frontend_update_formats(struct sun4i_frontend *frontend,
 			   SUN4I_FRONTEND_BYPASS_CSC_EN);
 
 	regmap_write(frontend->regs, SUN4I_FRONTEND_INPUT_FMT_REG,
-		     SUN4I_FRONTEND_INPUT_FMT_DATA_MOD(1) |
+		     SUN4I_FRONTEND_INPUT_FMT_DATA_MOD(in_mod_val) |
 		     SUN4I_FRONTEND_INPUT_FMT_DATA_FMT(in_fmt_val) |
-		     SUN4I_FRONTEND_INPUT_FMT_PS(1));
+		     SUN4I_FRONTEND_INPUT_FMT_PS(in_ps_val));
 
 	/*
 	 * TODO: It look like the A31 and A80 at least will need the
-- 
2.19.1

Re: [PATCH v2 17/43] drm/sun4i: frontend: Add helpers for input data mode and pixel sequence

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 479 bytes --]

On Fri, Nov 23, 2018 at 10:24:49AM +0100, Paul Kocialkowski wrote:
> This introduces new helpers for retrieving the input data mode and pixel
> sequence register field values based on the DRM format instead of
> hardcoding these. This makes it easier to add support for more formats.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 18/43] drm/sun4i: frontend: Add proper definitions for format registers

[Paul Kocialkowski]

This introduces proper definitions for the input and output format
configuration registers instead of a macro and raw values in the code,
with the intent to increase code readability and reduce indirections.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 14 ++++++--------
 drivers/gpu/drm/sun4i/sun4i_frontend.h |  8 ++++----
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index bb5977a466f3..a305b731b042 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -108,7 +108,7 @@ static int sun4i_frontend_drm_format_to_input_fmt(uint32_t fmt, u32 *val)
 {
 	switch (fmt) {
 	case DRM_FORMAT_XRGB8888:
-		*val = 5;
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_RGB;
 		return 0;
 
 	default:
@@ -120,7 +120,7 @@ static int sun4i_frontend_drm_format_to_input_mode(uint32_t fmt, u32 *val)
 {
 	switch (fmt) {
 	case DRM_FORMAT_XRGB8888:
-		*val = 1;
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_PACKED;
 		return 0;
 
 	default:
@@ -132,7 +132,7 @@ static int sun4i_frontend_drm_format_to_input_sequence(uint32_t fmt, u32 *val)
 {
 	switch (fmt) {
 	case DRM_FORMAT_XRGB8888:
-		*val = 1;
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_PS_XRGB;
 		return 0;
 
 	default:
@@ -144,7 +144,7 @@ static int sun4i_frontend_drm_format_to_output_fmt(uint32_t fmt, u32 *val)
 {
 	switch (fmt) {
 	case DRM_FORMAT_XRGB8888:
-		*val = 2;
+		*val = SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT_XRGB8888;
 		return 0;
 
 	default:
@@ -218,9 +218,7 @@ int sun4i_frontend_update_formats(struct sun4i_frontend *frontend,
 			   SUN4I_FRONTEND_BYPASS_CSC_EN);
 
 	regmap_write(frontend->regs, SUN4I_FRONTEND_INPUT_FMT_REG,
-		     SUN4I_FRONTEND_INPUT_FMT_DATA_MOD(in_mod_val) |
-		     SUN4I_FRONTEND_INPUT_FMT_DATA_FMT(in_fmt_val) |
-		     SUN4I_FRONTEND_INPUT_FMT_PS(in_ps_val));
+		     in_mod_val | in_fmt_val | in_ps_val);
 
 	/*
 	 * TODO: It look like the A31 and A80 at least will need the
@@ -228,7 +226,7 @@ int sun4i_frontend_update_formats(struct sun4i_frontend *frontend,
 	 * ARGB8888).
 	 */
 	regmap_write(frontend->regs, SUN4I_FRONTEND_OUTPUT_FMT_REG,
-		     SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT(out_fmt_val));
+		     out_fmt_val);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.h b/drivers/gpu/drm/sun4i/sun4i_frontend.h
index a9cb908ced16..56bd5a3f9723 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.h
@@ -26,12 +26,12 @@
 #define SUN4I_FRONTEND_LINESTRD0_REG		0x040
 
 #define SUN4I_FRONTEND_INPUT_FMT_REG		0x04c
-#define SUN4I_FRONTEND_INPUT_FMT_DATA_MOD(mod)		((mod) << 8)
-#define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT(fmt)		((fmt) << 4)
-#define SUN4I_FRONTEND_INPUT_FMT_PS(ps)			(ps)
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_PACKED	(1 << 8)
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_RGB		(5 << 4)
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_XRGB		1
 
 #define SUN4I_FRONTEND_OUTPUT_FMT_REG		0x05c
-#define SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT(fmt)		(fmt)
+#define SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT_XRGB8888	2
 
 #define SUN4I_FRONTEND_CH0_INSIZE_REG		0x100
 #define SUN4I_FRONTEND_INSIZE(h, w)			((((h) - 1) << 16) | (((w) - 1)))
-- 
2.19.1

Re: [PATCH v2 18/43] drm/sun4i: frontend: Add proper definitions for format registers

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 476 bytes --]

On Fri, Nov 23, 2018 at 10:24:50AM +0100, Paul Kocialkowski wrote:
> This introduces proper definitions for the input and output format
> configuration registers instead of a macro and raw values in the code,
> with the intent to increase code readability and reduce indirections.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 19/43] drm/sun4i: frontend: Determine input mode based on the number of planes

[Paul Kocialkowski]

Use the number of planes associated with the DRM format to determine the
input mode configuration instead of the format iteself. This way, the
helper can be used for all packed formats without future changes.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index a305b731b042..a16697b0eac5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -118,14 +118,12 @@ static int sun4i_frontend_drm_format_to_input_fmt(uint32_t fmt, u32 *val)
 
 static int sun4i_frontend_drm_format_to_input_mode(uint32_t fmt, u32 *val)
 {
-	switch (fmt) {
-	case DRM_FORMAT_XRGB8888:
+	if (drm_format_num_planes(fmt) == 1)
 		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_PACKED;
-		return 0;
-
-	default:
+	else
 		return -EINVAL;
-	}
+
+	return 0;
 }
 
 static int sun4i_frontend_drm_format_to_input_sequence(uint32_t fmt, u32 *val)
-- 
2.19.1

Re: [PATCH v2 19/43] drm/sun4i: frontend: Determine input mode based on the number of planes

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 477 bytes --]

On Fri, Nov 23, 2018 at 10:24:51AM +0100, Paul Kocialkowski wrote:
> Use the number of planes associated with the DRM format to determine the
> input mode configuration instead of the format iteself. This way, the
> helper can be used for all packed formats without future changes.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 20/43] drm/sun4i: frontend: Determine input format based on colorspace

[Paul Kocialkowski]

Since all the RGB input formats have the same value for the DATA_FMT
field of the INPUT_FMT register, we can group them when the format is
known to be RGB. Here, we assume that a non-YUV format is RGB, because
the hardware does not support any other colorspace than RGB and YUV.
Thus, we can use the associated common DRM helper.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index a16697b0eac5..7a8efbd516ac 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -106,14 +106,12 @@ EXPORT_SYMBOL(sun4i_frontend_update_buffer);
 
 static int sun4i_frontend_drm_format_to_input_fmt(uint32_t fmt, u32 *val)
 {
-	switch (fmt) {
-	case DRM_FORMAT_XRGB8888:
+	if (!drm_format_is_yuv(fmt))
 		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_RGB;
-		return 0;
-
-	default:
+	else
 		return -EINVAL;
-	}
+
+	return 0;
 }
 
 static int sun4i_frontend_drm_format_to_input_mode(uint32_t fmt, u32 *val)
-- 
2.19.1

[PATCH v2 21/43] drm/sun4i: frontend: Add support for the BGRX8888 input format

[Paul Kocialkowski]

This introduces support for the BGRX8888 input format for the frontend,
with its associated pixel sequence value definition. Other fields are
already configured correctly as they no longer depend on the format's
fourcc directly.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 5 +++++
 drivers/gpu/drm/sun4i/sun4i_frontend.h | 1 +
 drivers/gpu/drm/sun4i/sun4i_layer.c    | 1 +
 3 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index 7a8efbd516ac..e579fcf63ab4 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -127,6 +127,10 @@ static int sun4i_frontend_drm_format_to_input_mode(uint32_t fmt, u32 *val)
 static int sun4i_frontend_drm_format_to_input_sequence(uint32_t fmt, u32 *val)
 {
 	switch (fmt) {
+	case DRM_FORMAT_BGRX8888:
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_PS_BGRX;
+		return 0;
+
 	case DRM_FORMAT_XRGB8888:
 		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_PS_XRGB;
 		return 0;
@@ -149,6 +153,7 @@ static int sun4i_frontend_drm_format_to_output_fmt(uint32_t fmt, u32 *val)
 }
 
 static const uint32_t sun4i_frontend_formats[] = {
+	DRM_FORMAT_BGRX8888,
 	DRM_FORMAT_XRGB8888,
 };
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.h b/drivers/gpu/drm/sun4i/sun4i_frontend.h
index 56bd5a3f9723..2ee1bccf378a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.h
@@ -28,6 +28,7 @@
 #define SUN4I_FRONTEND_INPUT_FMT_REG		0x04c
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_PACKED	(1 << 8)
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_RGB		(5 << 4)
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_BGRX		0
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_XRGB		1
 
 #define SUN4I_FRONTEND_OUTPUT_FMT_REG		0x05c
diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index c3368b17e5f1..6155eca6b1bd 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -131,6 +131,7 @@ static const uint32_t sun4i_layer_formats[] = {
 	DRM_FORMAT_ARGB8888,
 	DRM_FORMAT_ARGB4444,
 	DRM_FORMAT_ARGB1555,
+	DRM_FORMAT_BGRX8888,
 	DRM_FORMAT_RGBA5551,
 	DRM_FORMAT_RGBA4444,
 	DRM_FORMAT_RGB888,
-- 
2.19.1

Re: [PATCH v2 21/43] drm/sun4i: frontend: Add support for the BGRX8888 input format

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 500 bytes --]

On Fri, Nov 23, 2018 at 10:24:53AM +0100, Paul Kocialkowski wrote:
> This introduces support for the BGRX8888 input format for the frontend,
> with its associated pixel sequence value definition. Other fields are
> already configured correctly as they no longer depend on the format's
> fourcc directly.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 22/43] drm/sun4i: frontend: Add support for the BGRX8888 output format

[Paul Kocialkowski]

This introduces support for the BGRX8888 output format for the frontend,
with its associated output format value definition.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 4 ++++
 drivers/gpu/drm/sun4i/sun4i_frontend.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index e579fcf63ab4..7fa8fb3eb7aa 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -143,6 +143,10 @@ static int sun4i_frontend_drm_format_to_input_sequence(uint32_t fmt, u32 *val)
 static int sun4i_frontend_drm_format_to_output_fmt(uint32_t fmt, u32 *val)
 {
 	switch (fmt) {
+	case DRM_FORMAT_BGRX8888:
+		*val = SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT_BGRX8888;
+		return 0;
+
 	case DRM_FORMAT_XRGB8888:
 		*val = SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT_XRGB8888;
 		return 0;
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.h b/drivers/gpu/drm/sun4i/sun4i_frontend.h
index 2ee1bccf378a..800a193eebc2 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.h
@@ -32,6 +32,7 @@
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_XRGB		1
 
 #define SUN4I_FRONTEND_OUTPUT_FMT_REG		0x05c
+#define SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT_BGRX8888	1
 #define SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT_XRGB8888	2
 
 #define SUN4I_FRONTEND_CH0_INSIZE_REG		0x100
-- 
2.19.1

Re: [PATCH v2 22/43] drm/sun4i: frontend: Add support for the BGRX8888 output format

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 390 bytes --]

On Fri, Nov 23, 2018 at 10:24:54AM +0100, Paul Kocialkowski wrote:
> This introduces support for the BGRX8888 output format for the frontend,
> with its associated output format value definition.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 23/43] drm/sun4i: backend: Detail the YUV to RGB values coding explanation

[Paul Kocialkowski]

From: Paul Kocialkowski <contact@paulk.fr>

The values in the BT601 YUV to RGB colorspace translation are not
simply coded as multiples, but rather as fixed-point signed fractional
values on a given number of bits. Add an explanation about that.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index a39d8b75d76a..986bdb64521c 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -48,8 +48,12 @@ static const u32 sunxi_rgb2yuv_coef[12] = {
 /*
  * These coefficients are taken from the A33 BSP from Allwinner.
  *
- * The formula is for each component, each coefficient being multiplied by
- * 1024 and each constant being multiplied by 16:
+ * The first three values of each row are coded as 13-bit signed fixed-point
+ * numbers, with 10 bits for the fractional part. The fourth value is a
+ * constant coded as a 14-bit signed fixed-point number with 4 bits for the
+ * fractional part.
+ *
+ * The values in table order give the following colorspace translation:
  * G = 1.164 * Y - 0.391 * U - 0.813 * V + 135
  * R = 1.164 * Y + 1.596 * V - 222
  * B = 1.164 * Y + 2.018 * U + 276
-- 
2.19.1

Re: [PATCH v2 23/43] drm/sun4i: backend: Detail the YUV to RGB values coding explanation

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]

On Fri, Nov 23, 2018 at 10:24:55AM +0100, Paul Kocialkowski wrote:
> From: Paul Kocialkowski <contact@paulk.fr>
> 
> The values in the BT601 YUV to RGB colorspace translation are not
> simply coded as multiples, but rather as fixed-point signed fractional
> values on a given number of bits. Add an explanation about that.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 24/43] drm/sun4i: frontend: Configure and enable YUV to RGB CSC when needed

[Paul Kocialkowski]

In prevision of adding support for YUV formats, set the YUV to RGB
colorspace conversion coefficients if required and don't bypass the
CSC engine when converting.

The BT601 coefficients from the A33 BSP are copied over from the backend
code. Because of module inter-dependency, we can't have the frontend use
these coefficients from the backend directly.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 39 ++++++++++++++++++++++++--
 drivers/gpu/drm/sun4i/sun4i_frontend.h |  2 ++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index 7fa8fb3eb7aa..5626334c6e87 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -48,6 +48,28 @@ static const u32 sun4i_frontend_horz_coef[64] = {
 	0x03ff0000, 0x0000fd41, 0x01ff0000, 0x0000fe42,
 };
 
+/*
+ * These coefficients are taken from the A33 BSP from Allwinner.
+ *
+ * The first three values of each row are coded as 13-bit signed fixed-point
+ * numbers, with 10 bits for the fractional part. The fourth value is a
+ * constant coded as a 14-bit signed fixed-point number with 4 bits for the
+ * fractional part.
+ *
+ * The values in table order give the following colorspace translation:
+ * G = 1.164 * Y - 0.391 * U - 0.813 * V + 135
+ * R = 1.164 * Y + 1.596 * V - 222
+ * B = 1.164 * Y + 2.018 * U + 276
+ *
+ * This seems to be a conversion from Y[16:235] UV[16:240] to RGB[0:255],
+ * following the BT601 spec.
+ */
+static const u32 sunxi_bt601_yuv2rgb_coef[12] = {
+	0x000004a7, 0x00001e6f, 0x00001cbf, 0x00000877,
+	0x000004a7, 0x00000000, 0x00000662, 0x00003211,
+	0x000004a7, 0x00000812, 0x00000000, 0x00002eb1,
+};
+
 static void sun4i_frontend_scaler_init(struct sun4i_frontend *frontend)
 {
 	int i;
@@ -181,6 +203,8 @@ int sun4i_frontend_update_formats(struct sun4i_frontend *frontend,
 	uint32_t format = fb->format->format;
 	u32 out_fmt_val;
 	u32 in_fmt_val, in_mod_val, in_ps_val;
+	unsigned int i;
+	u32 bypass;
 	int ret;
 
 	ret = sun4i_frontend_drm_format_to_input_fmt(format, &in_fmt_val);
@@ -218,9 +242,20 @@ int sun4i_frontend_update_formats(struct sun4i_frontend *frontend,
 	regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_VERTPHASE1_REG, 0x400);
 	regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_VERTPHASE1_REG, 0x400);
 
+	if (fb->format->is_yuv && !drm_format_is_yuv(out_fmt)) {
+		/* Setup the CSC engine for YUV to RGB conversion. */
+		bypass = 0;
+
+		for (i = 0; i < ARRAY_SIZE(sunxi_bt601_yuv2rgb_coef); i++)
+			regmap_write(frontend->regs,
+				     SUN4I_FRONTEND_CSC_COEF_REG(i),
+				     sunxi_bt601_yuv2rgb_coef[i]);
+	} else {
+		bypass = SUN4I_FRONTEND_BYPASS_CSC_EN;
+	}
+
 	regmap_update_bits(frontend->regs, SUN4I_FRONTEND_BYPASS_REG,
-			   SUN4I_FRONTEND_BYPASS_CSC_EN,
-			   SUN4I_FRONTEND_BYPASS_CSC_EN);
+			   SUN4I_FRONTEND_BYPASS_CSC_EN, bypass);
 
 	regmap_write(frontend->regs, SUN4I_FRONTEND_INPUT_FMT_REG,
 		     in_mod_val | in_fmt_val | in_ps_val);
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.h b/drivers/gpu/drm/sun4i/sun4i_frontend.h
index 800a193eebc2..fb097c34e160 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.h
@@ -35,6 +35,8 @@
 #define SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT_BGRX8888	1
 #define SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT_XRGB8888	2
 
+#define SUN4I_FRONTEND_CSC_COEF_REG(c)		(0x070 + (0x4 * (c)))
+
 #define SUN4I_FRONTEND_CH0_INSIZE_REG		0x100
 #define SUN4I_FRONTEND_INSIZE(h, w)			((((h) - 1) << 16) | (((w) - 1)))
 
-- 
2.19.1

[PATCH v2 25/43] drm/sun4i: frontend: Apply format sub-sampling to CH1 dimensions

[Paul Kocialkowski]

The frontend comes with two "channels", that can be configured
independently. When used in YUV mode, the first channel (CH0) represents
the luminance component while the second channel (CH1) represents the
chrominance. In RGB mode, both have to be configured the same way.

Use variables (with the YUV terminology) for each channel's
dimensions, calculating the chroma dimensions from the luma dimensions
and the sub-sampling factors from the format description.

Since the configured size only has pixel precision, the fractional
fixed-point part of the source size is dropped for both components to
ensure that the scaling factors are accurate.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index 5626334c6e87..4700133b96e0 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -276,16 +276,24 @@ void sun4i_frontend_update_coord(struct sun4i_frontend *frontend,
 				 struct drm_plane *plane)
 {
 	struct drm_plane_state *state = plane->state;
+	struct drm_framebuffer *fb = state->fb;
+	uint32_t luma_width, luma_height;
+	uint32_t chroma_width, chroma_height;
 
 	/* Set height and width */
 	DRM_DEBUG_DRIVER("Frontend size W: %u H: %u\n",
 			 state->crtc_w, state->crtc_h);
+
+	luma_width = state->src_w >> 16;
+	luma_height = state->src_h >> 16;
+
+	chroma_width = DIV_ROUND_UP(luma_width, fb->format->hsub);
+	chroma_height = DIV_ROUND_UP(luma_height, fb->format->vsub);
+
 	regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_INSIZE_REG,
-		     SUN4I_FRONTEND_INSIZE(state->src_h >> 16,
-					   state->src_w >> 16));
+		     SUN4I_FRONTEND_INSIZE(luma_height, luma_width));
 	regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_INSIZE_REG,
-		     SUN4I_FRONTEND_INSIZE(state->src_h >> 16,
-					   state->src_w >> 16));
+		     SUN4I_FRONTEND_INSIZE(chroma_height, chroma_width));
 
 	regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_OUTSIZE_REG,
 		     SUN4I_FRONTEND_OUTSIZE(state->crtc_h, state->crtc_w));
@@ -293,14 +301,14 @@ void sun4i_frontend_update_coord(struct sun4i_frontend *frontend,
 		     SUN4I_FRONTEND_OUTSIZE(state->crtc_h, state->crtc_w));
 
 	regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_HORZFACT_REG,
-		     state->src_w / state->crtc_w);
+		     (luma_width << 16) / state->crtc_w);
 	regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_HORZFACT_REG,
-		     state->src_w / state->crtc_w);
+		     (chroma_width << 16) / state->crtc_w);
 
 	regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_VERTFACT_REG,
-		     state->src_h / state->crtc_h);
+		     (luma_height << 16) / state->crtc_h);
 	regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_VERTFACT_REG,
-		     state->src_h / state->crtc_h);
+		     (chroma_height << 16) / state->crtc_h);
 
 	regmap_write_bits(frontend->regs, SUN4I_FRONTEND_FRM_CTRL_REG,
 			  SUN4I_FRONTEND_FRM_CTRL_REG_RDY,
-- 
2.19.1

Re: [PATCH v2 25/43] drm/sun4i: frontend: Apply format sub-sampling to CH1 dimensions

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

On Fri, Nov 23, 2018 at 10:24:57AM +0100, Paul Kocialkowski wrote:
> The frontend comes with two "channels", that can be configured
> independently. When used in YUV mode, the first channel (CH0) represents
> the luminance component while the second channel (CH1) represents the
> chrominance. In RGB mode, both have to be configured the same way.
> 
> Use variables (with the YUV terminology) for each channel's
> dimensions, calculating the chroma dimensions from the luma dimensions
> and the sub-sampling factors from the format description.
> 
> Since the configured size only has pixel precision, the fractional
> fixed-point part of the source size is dropped for both components to
> ensure that the scaling factors are accurate.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 26/43] drm/sun4i: frontend: Add support for packed YUV422 input formats

[Paul Kocialkowski]

This introduces support for packed YUV formats with 4:2:2 sampling using
the frontend. Definitions are introduced for the data format and pixel
sequence input format register values.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 22 ++++++++++++++++++++++
 drivers/gpu/drm/sun4i/sun4i_frontend.h |  5 +++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index 4700133b96e0..f443e8e1d8ec 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -130,6 +130,8 @@ static int sun4i_frontend_drm_format_to_input_fmt(uint32_t fmt, u32 *val)
 {
 	if (!drm_format_is_yuv(fmt))
 		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_RGB;
+	else if (drm_format_is_yuv_sampling_422(fmt))
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV422;
 	else
 		return -EINVAL;
 
@@ -153,10 +155,26 @@ static int sun4i_frontend_drm_format_to_input_sequence(uint32_t fmt, u32 *val)
 		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_PS_BGRX;
 		return 0;
 
+	case DRM_FORMAT_UYVY:
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_PS_UYVY;
+		return 0;
+
+	case DRM_FORMAT_VYUY:
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_PS_VYUY;
+		return 0;
+
 	case DRM_FORMAT_XRGB8888:
 		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_PS_XRGB;
 		return 0;
 
+	case DRM_FORMAT_YUYV:
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_PS_YUYV;
+		return 0;
+
+	case DRM_FORMAT_YVYU:
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_PS_YVYU;
+		return 0;
+
 	default:
 		return -EINVAL;
 	}
@@ -180,7 +198,11 @@ static int sun4i_frontend_drm_format_to_output_fmt(uint32_t fmt, u32 *val)
 
 static const uint32_t sun4i_frontend_formats[] = {
 	DRM_FORMAT_BGRX8888,
+	DRM_FORMAT_UYVY,
+	DRM_FORMAT_VYUY,
 	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_YUYV,
+	DRM_FORMAT_YVYU,
 };
 
 bool sun4i_frontend_format_is_supported(uint32_t fmt)
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.h b/drivers/gpu/drm/sun4i/sun4i_frontend.h
index fb097c34e160..399755939a28 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.h
@@ -27,7 +27,12 @@
 
 #define SUN4I_FRONTEND_INPUT_FMT_REG		0x04c
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_PACKED	(1 << 8)
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV422	(1 << 4)
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_RGB		(5 << 4)
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_UYVY		0
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_YUYV		1
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_VYUY		2
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_YVYU		3
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_BGRX		0
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_XRGB		1
 
-- 
2.19.1

[PATCH v2 27/43] drm/sun4i: frontend: Add support for semi-planar YUV input formats

[Paul Kocialkowski]

Semi-planar YUV formats use two distinct planes, one for luminance and
one for chrominance. To add support for them, we need to configure the
second line stride and buffer address registers to setup the second YUV
plane.

New definitions are introduced to configure the input format register
for the YUV420 and YUV422 semi-planar formats.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 52 +++++++++++++++++++++++---
 drivers/gpu/drm/sun4i/sun4i_frontend.h |  6 +++
 drivers/gpu/drm/sun4i/sun4i_layer.c    |  4 ++
 3 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index f443e8e1d8ec..e31438e5cabd 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -118,11 +118,23 @@ void sun4i_frontend_update_buffer(struct sun4i_frontend *frontend,
 	regmap_write(frontend->regs, SUN4I_FRONTEND_LINESTRD0_REG,
 		     fb->pitches[0]);
 
+	if (fb->format->num_planes > 1)
+		regmap_write(frontend->regs, SUN4I_FRONTEND_LINESTRD1_REG,
+			     fb->pitches[1]);
+
 	/* Set the physical address of the buffer in memory */
 	paddr = drm_fb_cma_get_gem_addr(fb, state, 0);
 	paddr -= PHYS_OFFSET;
-	DRM_DEBUG_DRIVER("Setting buffer address to %pad\n", &paddr);
+	DRM_DEBUG_DRIVER("Setting buffer #0 address to %pad\n", &paddr);
 	regmap_write(frontend->regs, SUN4I_FRONTEND_BUF_ADDR0_REG, paddr);
+
+	if (fb->format->num_planes > 1) {
+		paddr = drm_fb_cma_get_gem_addr(fb, state, 1);
+		paddr -= PHYS_OFFSET;
+		DRM_DEBUG_DRIVER("Setting buffer #1 address to %pad\n", &paddr);
+		regmap_write(frontend->regs, SUN4I_FRONTEND_BUF_ADDR1_REG,
+			     paddr);
+	}
 }
 EXPORT_SYMBOL(sun4i_frontend_update_buffer);
 
@@ -130,6 +142,8 @@ static int sun4i_frontend_drm_format_to_input_fmt(uint32_t fmt, u32 *val)
 {
 	if (!drm_format_is_yuv(fmt))
 		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_RGB;
+	else if (drm_format_is_yuv_sampling_420(fmt))
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV420;
 	else if (drm_format_is_yuv_sampling_422(fmt))
 		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV422;
 	else
@@ -140,12 +154,20 @@ static int sun4i_frontend_drm_format_to_input_fmt(uint32_t fmt, u32 *val)
 
 static int sun4i_frontend_drm_format_to_input_mode(uint32_t fmt, u32 *val)
 {
-	if (drm_format_num_planes(fmt) == 1)
+	int num_planes = drm_format_num_planes(fmt);
+
+	switch (num_planes) {
+	case 1:
 		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_PACKED;
-	else
-		return -EINVAL;
+		return 0;
 
-	return 0;
+	case 2:
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_SEMIPLANAR;
+		return 0;
+
+	default:
+		return -EINVAL;
+	}
 }
 
 static int sun4i_frontend_drm_format_to_input_sequence(uint32_t fmt, u32 *val)
@@ -155,6 +177,22 @@ static int sun4i_frontend_drm_format_to_input_sequence(uint32_t fmt, u32 *val)
 		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_PS_BGRX;
 		return 0;
 
+	case DRM_FORMAT_NV12:
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_PS_UV;
+		return 0;
+
+	case DRM_FORMAT_NV16:
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_PS_UV;
+		return 0;
+
+	case DRM_FORMAT_NV21:
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_PS_VU;
+		return 0;
+
+	case DRM_FORMAT_NV61:
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_PS_VU;
+		return 0;
+
 	case DRM_FORMAT_UYVY:
 		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_PS_UYVY;
 		return 0;
@@ -198,6 +236,10 @@ static int sun4i_frontend_drm_format_to_output_fmt(uint32_t fmt, u32 *val)
 
 static const uint32_t sun4i_frontend_formats[] = {
 	DRM_FORMAT_BGRX8888,
+	DRM_FORMAT_NV12,
+	DRM_FORMAT_NV16,
+	DRM_FORMAT_NV21,
+	DRM_FORMAT_NV61,
 	DRM_FORMAT_UYVY,
 	DRM_FORMAT_VYUY,
 	DRM_FORMAT_XRGB8888,
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.h b/drivers/gpu/drm/sun4i/sun4i_frontend.h
index 399755939a28..7d01b4f09b75 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.h
@@ -22,17 +22,23 @@
 #define SUN4I_FRONTEND_BYPASS_CSC_EN			BIT(1)
 
 #define SUN4I_FRONTEND_BUF_ADDR0_REG		0x020
+#define SUN4I_FRONTEND_BUF_ADDR1_REG		0x024
 
 #define SUN4I_FRONTEND_LINESTRD0_REG		0x040
+#define SUN4I_FRONTEND_LINESTRD1_REG		0x044
 
 #define SUN4I_FRONTEND_INPUT_FMT_REG		0x04c
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_PACKED	(1 << 8)
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_SEMIPLANAR	(2 << 8)
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV422	(1 << 4)
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV420	(2 << 4)
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_RGB		(5 << 4)
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_UYVY		0
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_YUYV		1
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_VYUY		2
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_YVYU		3
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_UV		0
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_VU		1
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_BGRX		0
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_XRGB		1
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index 6155eca6b1bd..80c718f06ce8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -136,6 +136,10 @@ static const uint32_t sun4i_layer_formats[] = {
 	DRM_FORMAT_RGBA4444,
 	DRM_FORMAT_RGB888,
 	DRM_FORMAT_RGB565,
+	DRM_FORMAT_NV12,
+	DRM_FORMAT_NV16,
+	DRM_FORMAT_NV21,
+	DRM_FORMAT_NV61,
 	DRM_FORMAT_UYVY,
 	DRM_FORMAT_VYUY,
 	DRM_FORMAT_XRGB8888,
-- 
2.19.1

Re: [PATCH v2 27/43] drm/sun4i: frontend: Add support for semi-planar YUV input formats

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3001 bytes --]

On Fri, Nov 23, 2018 at 10:24:59AM +0100, Paul Kocialkowski wrote:
> Semi-planar YUV formats use two distinct planes, one for luminance and
> one for chrominance. To add support for them, we need to configure the
> second line stride and buffer address registers to setup the second YUV
> plane.
> 
> New definitions are introduced to configure the input format register
> for the YUV420 and YUV422 semi-planar formats.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  drivers/gpu/drm/sun4i/sun4i_frontend.c | 52 +++++++++++++++++++++++---
>  drivers/gpu/drm/sun4i/sun4i_frontend.h |  6 +++
>  drivers/gpu/drm/sun4i/sun4i_layer.c    |  4 ++
>  3 files changed, 57 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
> index f443e8e1d8ec..e31438e5cabd 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
> @@ -118,11 +118,23 @@ void sun4i_frontend_update_buffer(struct sun4i_frontend *frontend,
>  	regmap_write(frontend->regs, SUN4I_FRONTEND_LINESTRD0_REG,
>  		     fb->pitches[0]);
>  
> +	if (fb->format->num_planes > 1)
> +		regmap_write(frontend->regs, SUN4I_FRONTEND_LINESTRD1_REG,
> +			     fb->pitches[1]);
> +
>  	/* Set the physical address of the buffer in memory */
>  	paddr = drm_fb_cma_get_gem_addr(fb, state, 0);
>  	paddr -= PHYS_OFFSET;
> -	DRM_DEBUG_DRIVER("Setting buffer address to %pad\n", &paddr);
> +	DRM_DEBUG_DRIVER("Setting buffer #0 address to %pad\n", &paddr);
>  	regmap_write(frontend->regs, SUN4I_FRONTEND_BUF_ADDR0_REG, paddr);
> +
> +	if (fb->format->num_planes > 1) {
> +		paddr = drm_fb_cma_get_gem_addr(fb, state, 1);
> +		paddr -= PHYS_OFFSET;
> +		DRM_DEBUG_DRIVER("Setting buffer #1 address to %pad\n", &paddr);
> +		regmap_write(frontend->regs, SUN4I_FRONTEND_BUF_ADDR1_REG,
> +			     paddr);
> +	}
>  }
>  EXPORT_SYMBOL(sun4i_frontend_update_buffer);
>  
> @@ -130,6 +142,8 @@ static int sun4i_frontend_drm_format_to_input_fmt(uint32_t fmt, u32 *val)
>  {
>  	if (!drm_format_is_yuv(fmt))
>  		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_RGB;
> +	else if (drm_format_is_yuv_sampling_420(fmt))
> +		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV420;
>  	else if (drm_format_is_yuv_sampling_422(fmt))
>  		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV422;
>  	else
> @@ -140,12 +154,20 @@ static int sun4i_frontend_drm_format_to_input_fmt(uint32_t fmt, u32 *val)
>  
>  static int sun4i_frontend_drm_format_to_input_mode(uint32_t fmt, u32 *val)
>  {
> -	if (drm_format_num_planes(fmt) == 1)
> +	int num_planes = drm_format_num_planes(fmt);
> +
> +	switch (num_planes) {
> +	case 1:

You seem to use either the helper or retrieve the structure field
directly to access the number of planes. I'm fine either way, but this
should be consistent.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 28/43] drm/sun4i: frontend: Add support for planar YUV input formats

[Paul Kocialkowski]

Planar YUV formats come with 3 distinct planes, which requires
configuring the frontend line stride and address registers for the
third plane.

Our hardware only supports the YUV planes order and in order to support
formats with a YVU plane order, a helper is introduced to indicate
whether to invert the address of the two chroma planes.

Missing definitions for YUV411 and YUV444 input format configuration are
also introduced as support is added for these formats. For the input
sequence part, no configuration is required for planar YUV formats so
zero is returned in that case.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 54 +++++++++++++++++++++++++-
 drivers/gpu/drm/sun4i/sun4i_frontend.h |  5 +++
 drivers/gpu/drm/sun4i/sun4i_layer.c    |  8 ++++
 3 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index e31438e5cabd..001139417cf5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -106,12 +106,27 @@ void sun4i_frontend_exit(struct sun4i_frontend *frontend)
 }
 EXPORT_SYMBOL(sun4i_frontend_exit);
 
+static bool sun4i_frontend_format_chroma_requires_swap(uint32_t fmt)
+{
+	switch (fmt) {
+	case DRM_FORMAT_YVU411:
+	case DRM_FORMAT_YVU420:
+	case DRM_FORMAT_YVU422:
+	case DRM_FORMAT_YVU444:
+		return true;
+
+	default:
+		return false;
+	}
+}
+
 void sun4i_frontend_update_buffer(struct sun4i_frontend *frontend,
 				  struct drm_plane *plane)
 {
 	struct drm_plane_state *state = plane->state;
 	struct drm_framebuffer *fb = state->fb;
 	dma_addr_t paddr;
+	bool swap;
 
 	/* Set the line width */
 	DRM_DEBUG_DRIVER("Frontend stride: %d bytes\n", fb->pitches[0]);
@@ -122,6 +137,13 @@ void sun4i_frontend_update_buffer(struct sun4i_frontend *frontend,
 		regmap_write(frontend->regs, SUN4I_FRONTEND_LINESTRD1_REG,
 			     fb->pitches[1]);
 
+	if (fb->format->num_planes > 2)
+		regmap_write(frontend->regs, SUN4I_FRONTEND_LINESTRD2_REG,
+			     fb->pitches[2]);
+
+	/* Some planar formats require chroma channel swapping by hand. */
+	swap = sun4i_frontend_format_chroma_requires_swap(fb->format->format);
+
 	/* Set the physical address of the buffer in memory */
 	paddr = drm_fb_cma_get_gem_addr(fb, state, 0);
 	paddr -= PHYS_OFFSET;
@@ -129,12 +151,20 @@ void sun4i_frontend_update_buffer(struct sun4i_frontend *frontend,
 	regmap_write(frontend->regs, SUN4I_FRONTEND_BUF_ADDR0_REG, paddr);
 
 	if (fb->format->num_planes > 1) {
-		paddr = drm_fb_cma_get_gem_addr(fb, state, 1);
+		paddr = drm_fb_cma_get_gem_addr(fb, state, swap ? 2 : 1);
 		paddr -= PHYS_OFFSET;
 		DRM_DEBUG_DRIVER("Setting buffer #1 address to %pad\n", &paddr);
 		regmap_write(frontend->regs, SUN4I_FRONTEND_BUF_ADDR1_REG,
 			     paddr);
 	}
+
+	if (fb->format->num_planes > 2) {
+		paddr = drm_fb_cma_get_gem_addr(fb, state, swap ? 1 : 2);
+		paddr -= PHYS_OFFSET;
+		DRM_DEBUG_DRIVER("Setting buffer #2 address to %pad\n", &paddr);
+		regmap_write(frontend->regs, SUN4I_FRONTEND_BUF_ADDR2_REG,
+			     paddr);
+	}
 }
 EXPORT_SYMBOL(sun4i_frontend_update_buffer);
 
@@ -142,10 +172,14 @@ static int sun4i_frontend_drm_format_to_input_fmt(uint32_t fmt, u32 *val)
 {
 	if (!drm_format_is_yuv(fmt))
 		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_RGB;
+	else if (drm_format_is_yuv_sampling_411(fmt))
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV411;
 	else if (drm_format_is_yuv_sampling_420(fmt))
 		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV420;
 	else if (drm_format_is_yuv_sampling_422(fmt))
 		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV422;
+	else if (drm_format_is_yuv_sampling_444(fmt))
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV444;
 	else
 		return -EINVAL;
 
@@ -165,6 +199,10 @@ static int sun4i_frontend_drm_format_to_input_mode(uint32_t fmt, u32 *val)
 		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_SEMIPLANAR;
 		return 0;
 
+	case 3:
+		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_PLANAR;
+		return 0;
+
 	default:
 		return -EINVAL;
 	}
@@ -172,6 +210,12 @@ static int sun4i_frontend_drm_format_to_input_mode(uint32_t fmt, u32 *val)
 
 static int sun4i_frontend_drm_format_to_input_sequence(uint32_t fmt, u32 *val)
 {
+	/* Planar formats have an explicit input sequence. */
+	if (drm_format_is_yuv_planar(fmt)) {
+		*val = 0;
+		return 0;
+	}
+
 	switch (fmt) {
 	case DRM_FORMAT_BGRX8888:
 		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_PS_BGRX;
@@ -243,7 +287,15 @@ static const uint32_t sun4i_frontend_formats[] = {
 	DRM_FORMAT_UYVY,
 	DRM_FORMAT_VYUY,
 	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_YUV411,
+	DRM_FORMAT_YUV420,
+	DRM_FORMAT_YUV422,
+	DRM_FORMAT_YUV444,
 	DRM_FORMAT_YUYV,
+	DRM_FORMAT_YVU411,
+	DRM_FORMAT_YVU420,
+	DRM_FORMAT_YVU422,
+	DRM_FORMAT_YVU444,
 	DRM_FORMAT_YVYU,
 };
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.h b/drivers/gpu/drm/sun4i/sun4i_frontend.h
index 7d01b4f09b75..54ec71a3e2f4 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.h
@@ -23,15 +23,20 @@
 
 #define SUN4I_FRONTEND_BUF_ADDR0_REG		0x020
 #define SUN4I_FRONTEND_BUF_ADDR1_REG		0x024
+#define SUN4I_FRONTEND_BUF_ADDR2_REG		0x028
 
 #define SUN4I_FRONTEND_LINESTRD0_REG		0x040
 #define SUN4I_FRONTEND_LINESTRD1_REG		0x044
+#define SUN4I_FRONTEND_LINESTRD2_REG		0x048
 
 #define SUN4I_FRONTEND_INPUT_FMT_REG		0x04c
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_PLANAR	(0 << 8)
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_PACKED	(1 << 8)
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_SEMIPLANAR	(2 << 8)
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV444	(0 << 4)
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV422	(1 << 4)
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV420	(2 << 4)
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV411	(3 << 4)
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_RGB		(5 << 4)
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_UYVY		0
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_PS_YUYV		1
diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index 80c718f06ce8..2fb27819ad20 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -143,7 +143,15 @@ static const uint32_t sun4i_layer_formats[] = {
 	DRM_FORMAT_UYVY,
 	DRM_FORMAT_VYUY,
 	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_YUV411,
+	DRM_FORMAT_YUV420,
+	DRM_FORMAT_YUV422,
+	DRM_FORMAT_YUV444,
 	DRM_FORMAT_YUYV,
+	DRM_FORMAT_YVU411,
+	DRM_FORMAT_YVU420,
+	DRM_FORMAT_YVU422,
+	DRM_FORMAT_YVU444,
 	DRM_FORMAT_YVYU,
 };
 
-- 
2.19.1

[PATCH v2 29/43] drm/sun4i: Make pitch even for GEM dumb alloc as per hardware constraint

[Paul Kocialkowski]

Our hardware requires the pitch to be an even number when using YUV
formats with the frontend. Implement a driver-specific callback for GEM
dumb allocation that sets the pitch accordingly.

Since only the bpp is passed (and not the format), we cannot really
distinguish if this alignment is really required. Since it doesn't hurt
to align the pitch anyway, always do it.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_drv.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
index ef773d36baf0..ccdeae6299eb 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -28,6 +28,16 @@
 #include "sun4i_tcon.h"
 #include "sun8i_tcon_top.h"
 
+static int drm_sun4i_gem_dumb_create(struct drm_file *file_priv,
+				     struct drm_device *drm,
+				     struct drm_mode_create_dumb *args)
+{
+	/* The hardware only allows even pitches for YUV buffers. */
+	args->pitch = ALIGN(DIV_ROUND_UP(args->width * args->bpp, 8), 2);
+
+	return drm_gem_cma_dumb_create_internal(file_priv, drm, args);
+}
+
 DEFINE_DRM_GEM_CMA_FOPS(sun4i_drv_fops);
 
 static struct drm_driver sun4i_drv_driver = {
@@ -42,7 +52,7 @@ static struct drm_driver sun4i_drv_driver = {
 	.minor			= 0,
 
 	/* GEM Operations */
-	.dumb_create		= drm_gem_cma_dumb_create,
+	.dumb_create		= drm_sun4i_gem_dumb_create,
 	.gem_free_object_unlocked = drm_gem_cma_free_object,
 	.gem_vm_ops		= &drm_gem_cma_vm_ops,
 
-- 
2.19.1

Re: [PATCH v2 29/43] drm/sun4i: Make pitch even for GEM dumb alloc as per hardware constraint

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 651 bytes --]

On Fri, Nov 23, 2018 at 10:25:01AM +0100, Paul Kocialkowski wrote:
> Our hardware requires the pitch to be an even number when using YUV
> formats with the frontend. Implement a driver-specific callback for GEM
> dumb allocation that sets the pitch accordingly.
> 
> Since only the bpp is passed (and not the format), we cannot really
> distinguish if this alignment is really required. Since it doesn't hurt
> to align the pitch anyway, always do it.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 30/43] drm/fourcc: Add definitions for Allwinner vendor and VPU tiled format

[Paul Kocialkowski]

This introduces specific definitions for vendor Allwinner and its
associated tiled format modifier. This modifier is used for the output
format of the VPU, that can be imported directly with the display
engine hardware supported by the sun4i-drm driver.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 include/uapi/drm/drm_fourcc.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index e7e48f1f4a74..841535172507 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -237,6 +237,8 @@ extern "C" {
 #define DRM_FORMAT_MOD_VENDOR_VIVANTE 0x06
 #define DRM_FORMAT_MOD_VENDOR_BROADCOM 0x07
 #define DRM_FORMAT_MOD_VENDOR_ARM     0x08
+#define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
+
 /* add more to the end as needed */
 
 #define DRM_FORMAT_RESERVED	      ((1ULL << 56) - 1)
@@ -643,6 +645,20 @@ extern "C" {
  */
 #define AFBC_FORMAT_MOD_SC      (1ULL <<  9)
 
+/*
+ * Allwinner tiled modifier
+ *
+ * This tiling mode is implemented by the VPU found on all Allwinner platforms,
+ * codenamed sunxi. It is associated with a YUV format that uses either 2 or 3
+ * planes.
+ *
+ * With this tiling, the luminance samples are disposed in tiles representing
+ * 32x32 pixels and the chrominance samples in tiles representing 32x64 pixels.
+ * The pixel order in each tile is linear and the tiles are disposed linearly,
+ * both in row-major order.
+ */
+#define DRM_FORMAT_MOD_ALLWINNER_TILED fourcc_mod_code(ALLWINNER, 1)
+
 #if defined(__cplusplus)
 }
 #endif
-- 
2.19.1

Re: [PATCH v2 30/43] drm/fourcc: Add definitions for Allwinner vendor and VPU tiled format

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 563 bytes --]

On Fri, Nov 23, 2018 at 10:25:02AM +0100, Paul Kocialkowski wrote:
> This introduces specific definitions for vendor Allwinner and its
> associated tiled format modifier. This modifier is used for the output
> format of the VPU, that can be imported directly with the display
> engine hardware supported by the sun4i-drm driver.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime
-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 31/43] drm/sun4i: Add a dedicated ioctl call for allocating tiled buffers

[Paul Kocialkowski]

This introduces a dedicated ioctl for allocating buffers for the VPU
tiling mode. It allows setting up buffers that comply to the hardware
alignment requirements, by aligning the stride and height to 32 bytes.

Only YUV semiplanar and planar formats are allowed by the ioctl, as the
hardware does not support the tiling mode for other formats.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_drv.c | 89 +++++++++++++++++++++++++++++++
 include/uapi/drm/sun4i_drm.h      | 42 +++++++++++++++
 2 files changed, 131 insertions(+)
 create mode 100644 include/uapi/drm/sun4i_drm.h

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
index ccdeae6299eb..5ae32973cf34 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -21,6 +21,7 @@
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_of.h>
+#include <drm/sun4i_drm.h>
 
 #include "sun4i_drv.h"
 #include "sun4i_frontend.h"
@@ -28,6 +29,92 @@
 #include "sun4i_tcon.h"
 #include "sun8i_tcon_top.h"
 
+int drm_sun4i_gem_create_tiled(struct drm_device *drm, void *data,
+			       struct drm_file *file_priv)
+{
+	struct drm_sun4i_gem_create_tiled *args = data;
+	struct drm_gem_cma_object *cma_obj;
+	struct drm_gem_object *gem_obj;
+	uint32_t luma_stride, chroma_stride;
+	uint32_t luma_height, chroma_height;
+	uint32_t chroma_width;
+	const struct drm_format_info *info;
+	int ret;
+
+	info = drm_format_info(args->format);
+	if (!info)
+		return -EINVAL;
+
+	/* The tiled output format only applies to non-packed YUV formats. */
+	if (!info->is_yuv || info->num_planes == 1)
+		return -EINVAL;
+
+	memset(args->pitches, 0, sizeof(args->pitches));
+	memset(args->offsets, 0, sizeof(args->offsets));
+
+	/* Stride and height are aligned to 32 bytes for our tiled format. */
+	luma_stride = ALIGN(args->width, 32);
+	luma_height = ALIGN(args->height, 32);
+
+	chroma_width = args->width;
+
+	/* Semiplanar formats have both U and V data in their chroma plane. */
+	if (drm_format_info_is_yuv_semiplanar(info))
+		chroma_width *= 2;
+
+	chroma_stride = ALIGN(DIV_ROUND_UP(chroma_width, info->hsub), 32);
+	chroma_height = ALIGN(DIV_ROUND_UP(args->height, info->vsub), 32);
+
+	if (drm_format_info_is_yuv_semiplanar(info)) {
+		args->pitches[0] = luma_stride;
+		args->pitches[1] = chroma_stride;
+
+		args->offsets[0] = 0;
+		args->offsets[1] = luma_stride * luma_height;
+
+		args->size = luma_stride * luma_height +
+			     chroma_stride * chroma_height;
+	} else if (drm_format_info_is_yuv_planar(info)) {
+		args->pitches[0] = luma_stride;
+		args->pitches[1] = chroma_stride;
+		args->pitches[2] = chroma_stride;
+
+		args->offsets[0] = 0;
+		args->offsets[1] = luma_stride * luma_height;
+		args->offsets[2] = luma_stride * luma_height +
+				   chroma_stride * chroma_height;
+
+		args->size = luma_stride * luma_height +
+			     chroma_stride * chroma_height * 2;
+	} else {
+		/* No support for other formats in tiled mode. */
+		return -EINVAL;
+	}
+
+	cma_obj = drm_gem_cma_create(drm, args->size);
+	if (IS_ERR(cma_obj))
+		return PTR_ERR(cma_obj);
+
+	gem_obj = &cma_obj->base;
+
+	/*
+	 * allocate a id of idr table where the obj is registered
+	 * and handle has the id what user can see.
+	 */
+	ret = drm_gem_handle_create(file_priv, gem_obj, &args->handle);
+	/* drop reference from allocate - handle holds it now. */
+	drm_gem_object_put_unlocked(gem_obj);
+	if (ret)
+		return ret;
+
+	return PTR_ERR_OR_ZERO(cma_obj);
+}
+
+static const struct drm_ioctl_desc sun4i_drv_ioctls[] = {
+	DRM_IOCTL_DEF_DRV(SUN4I_GEM_CREATE_TILED, drm_sun4i_gem_create_tiled,
+			  DRM_UNLOCKED),
+};
+
 static int drm_sun4i_gem_dumb_create(struct drm_file *file_priv,
 				     struct drm_device *drm,
 				     struct drm_mode_create_dumb *args)
@@ -44,6 +131,8 @@ static struct drm_driver sun4i_drv_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC,
 
 	/* Generic Operations */
+	.ioctls			= sun4i_drv_ioctls,
+	.num_ioctls		= ARRAY_SIZE(sun4i_drv_ioctls),
 	.fops			= &sun4i_drv_fops,
 	.name			= "sun4i-drm",
 	.desc			= "Allwinner sun4i Display Engine",
diff --git a/include/uapi/drm/sun4i_drm.h b/include/uapi/drm/sun4i_drm.h
new file mode 100644
index 000000000000..2c77584b057b
--- /dev/null
+++ b/include/uapi/drm/sun4i_drm.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+/* sun4i_drm.h
+ *
+ * Copyright (C) 2018 Paul Kocialkowski <paul.kocialkowski@bootlin.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#ifndef _UAPI_SUN4I_DRM_H_
+#define _UAPI_SUN4I_DRM_H_
+
+#include "drm.h"
+
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
+struct drm_sun4i_gem_create_tiled {
+	__u32 height;
+	__u32 width;
+	__u32 format;
+	/* handle, offsets, pitches, size will be returned */
+	__u32 handle;
+	__u32 pitches[4];
+	__u32 offsets[4];
+	__u64 size;
+};
+
+#define DRM_SUN4I_GEM_CREATE_TILED	0x00
+
+#define DRM_IOCTL_SUN4I_GEM_CREATE_TILED \
+	DRM_IOWR(DRM_COMMAND_BASE + DRM_SUN4I_GEM_CREATE_TILED, \
+		 struct drm_sun4i_gem_create_tiled)
+
+#if defined(__cplusplus)
+}
+#endif
+
+#endif /* _UAPI_SUN4I_DRM_H_ */
-- 
2.19.1

Re: [PATCH v2 31/43] drm/sun4i: Add a dedicated ioctl call for allocating tiled buffers

[Brian Starkey]

Hi Paul,

On Fri, Nov 23, 2018 at 10:25:03AM +0100, Paul Kocialkowski wrote:
>This introduces a dedicated ioctl for allocating buffers for the VPU
>tiling mode. It allows setting up buffers that comply to the hardware
>alignment requirements, by aligning the stride and height to 32 bytes.
>
>Only YUV semiplanar and planar formats are allowed by the ioctl, as the
>hardware does not support the tiling mode for other formats.

What's the general feeling about a more generic version of this ioctl?
There doesn't seem to be anything Allwinner-specific in the ioctl
arguments.

It effectively boils down to:

	size = driver->get_fb_size_with_modifier(...);
 	cma_obj = drm_gem_cma_create(drm, size);

It would look exactly the same for Mali-DP, and probably others too.
Is it better to try and define something we can share instead of Arm
adding another nearly identical ioctl() later?

I think the minimal viable thing would be to just add modifiers to
your structure (I put them first because padding):

  struct drm_gem_create_with_modifiers {
  	__u64 modifiers[4];
  	__u32 height;
  	__u32 width;
  	__u32 format;
  	/* handle, offsets, pitches, size will be returned */
  	__u32 handle;
  	__u32 pitches[4];
  	__u32 offsets[4];
  	__u64 size;
  };

Thanks,
-Brian

>
>Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>---
> drivers/gpu/drm/sun4i/sun4i_drv.c | 89 +++++++++++++++++++++++++++++++
> include/uapi/drm/sun4i_drm.h      | 42 +++++++++++++++
> 2 files changed, 131 insertions(+)
> create mode 100644 include/uapi/drm/sun4i_drm.h
>
>diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
>index ccdeae6299eb..5ae32973cf34 100644
>--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
>+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
>@@ -21,6 +21,7 @@
> #include <drm/drm_gem_cma_helper.h>
> #include <drm/drm_fb_helper.h>
> #include <drm/drm_of.h>
>+#include <drm/sun4i_drm.h>
>
> #include "sun4i_drv.h"
> #include "sun4i_frontend.h"
>@@ -28,6 +29,92 @@
> #include "sun4i_tcon.h"
> #include "sun8i_tcon_top.h"
>
>+int drm_sun4i_gem_create_tiled(struct drm_device *drm, void *data,
>+			       struct drm_file *file_priv)
>+{
>+	struct drm_sun4i_gem_create_tiled *args = data;
>+	struct drm_gem_cma_object *cma_obj;
>+	struct drm_gem_object *gem_obj;
>+	uint32_t luma_stride, chroma_stride;
>+	uint32_t luma_height, chroma_height;
>+	uint32_t chroma_width;
>+	const struct drm_format_info *info;
>+	int ret;
>+
>+	info = drm_format_info(args->format);
>+	if (!info)
>+		return -EINVAL;
>+
>+	/* The tiled output format only applies to non-packed YUV formats. */
>+	if (!info->is_yuv || info->num_planes == 1)
>+		return -EINVAL;
>+
>+	memset(args->pitches, 0, sizeof(args->pitches));
>+	memset(args->offsets, 0, sizeof(args->offsets));
>+
>+	/* Stride and height are aligned to 32 bytes for our tiled format. */
>+	luma_stride = ALIGN(args->width, 32);
>+	luma_height = ALIGN(args->height, 32);
>+
>+	chroma_width = args->width;
>+
>+	/* Semiplanar formats have both U and V data in their chroma plane. */
>+	if (drm_format_info_is_yuv_semiplanar(info))
>+		chroma_width *= 2;
>+
>+	chroma_stride = ALIGN(DIV_ROUND_UP(chroma_width, info->hsub), 32);
>+	chroma_height = ALIGN(DIV_ROUND_UP(args->height, info->vsub), 32);
>+
>+	if (drm_format_info_is_yuv_semiplanar(info)) {
>+		args->pitches[0] = luma_stride;
>+		args->pitches[1] = chroma_stride;
>+
>+		args->offsets[0] = 0;
>+		args->offsets[1] = luma_stride * luma_height;
>+
>+		args->size = luma_stride * luma_height +
>+			     chroma_stride * chroma_height;
>+	} else if (drm_format_info_is_yuv_planar(info)) {
>+		args->pitches[0] = luma_stride;
>+		args->pitches[1] = chroma_stride;
>+		args->pitches[2] = chroma_stride;
>+
>+		args->offsets[0] = 0;
>+		args->offsets[1] = luma_stride * luma_height;
>+		args->offsets[2] = luma_stride * luma_height +
>+				   chroma_stride * chroma_height;
>+
>+		args->size = luma_stride * luma_height +
>+			     chroma_stride * chroma_height * 2;
>+	} else {
>+		/* No support for other formats in tiled mode. */
>+		return -EINVAL;
>+	}
>+
>+	cma_obj = drm_gem_cma_create(drm, args->size);
>+	if (IS_ERR(cma_obj))
>+		return PTR_ERR(cma_obj);
>+
>+	gem_obj = &cma_obj->base;
>+
>+	/*
>+	 * allocate a id of idr table where the obj is registered
>+	 * and handle has the id what user can see.
>+	 */
>+	ret = drm_gem_handle_create(file_priv, gem_obj, &args->handle);
>+	/* drop reference from allocate - handle holds it now. */
>+	drm_gem_object_put_unlocked(gem_obj);
>+	if (ret)
>+		return ret;
>+
>+	return PTR_ERR_OR_ZERO(cma_obj);
>+}
>+
>+static const struct drm_ioctl_desc sun4i_drv_ioctls[] = {
>+	DRM_IOCTL_DEF_DRV(SUN4I_GEM_CREATE_TILED, drm_sun4i_gem_create_tiled,
>+			  DRM_UNLOCKED),
>+};
>+
> static int drm_sun4i_gem_dumb_create(struct drm_file *file_priv,
> 				     struct drm_device *drm,
> 				     struct drm_mode_create_dumb *args)
>@@ -44,6 +131,8 @@ static struct drm_driver sun4i_drv_driver = {
> 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC,
>
> 	/* Generic Operations */
>+	.ioctls			= sun4i_drv_ioctls,
>+	.num_ioctls		= ARRAY_SIZE(sun4i_drv_ioctls),
> 	.fops			= &sun4i_drv_fops,
> 	.name			= "sun4i-drm",
> 	.desc			= "Allwinner sun4i Display Engine",
>diff --git a/include/uapi/drm/sun4i_drm.h b/include/uapi/drm/sun4i_drm.h
>new file mode 100644
>index 000000000000..2c77584b057b
>--- /dev/null
>+++ b/include/uapi/drm/sun4i_drm.h
>@@ -0,0 +1,42 @@
>+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
>+/* sun4i_drm.h
>+ *
>+ * Copyright (C) 2018 Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>+ *
>+ * This program is free software; you can redistribute  it and/or modify it
>+ * under  the terms of  the GNU General  Public License as published by the
>+ * Free Software Foundation;  either version 2 of the  License, or (at your
>+ * option) any later version.
>+ */
>+
>+#ifndef _UAPI_SUN4I_DRM_H_
>+#define _UAPI_SUN4I_DRM_H_
>+
>+#include "drm.h"
>+
>+#if defined(__cplusplus)
>+extern "C" {
>+#endif
>+
>+struct drm_sun4i_gem_create_tiled {
>+	__u32 height;
>+	__u32 width;
>+	__u32 format;
>+	/* handle, offsets, pitches, size will be returned */
>+	__u32 handle;
>+	__u32 pitches[4];
>+	__u32 offsets[4];
>+	__u64 size;
>+};
>+
>+#define DRM_SUN4I_GEM_CREATE_TILED	0x00
>+
>+#define DRM_IOCTL_SUN4I_GEM_CREATE_TILED \
>+	DRM_IOWR(DRM_COMMAND_BASE + DRM_SUN4I_GEM_CREATE_TILED, \
>+		 struct drm_sun4i_gem_create_tiled)
>+
>+#if defined(__cplusplus)
>+}
>+#endif
>+
>+#endif /* _UAPI_SUN4I_DRM_H_ */
>-- 
>2.19.1
>
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 31/43] drm/sun4i: Add a dedicated ioctl call for allocating tiled buffers

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 8004 bytes --]

Hi,

On Fri, 2018-11-23 at 11:30 +0000, Brian Starkey wrote:
> Hi Paul,
> 
> On Fri, Nov 23, 2018 at 10:25:03AM +0100, Paul Kocialkowski wrote:
> > This introduces a dedicated ioctl for allocating buffers for the VPU
> > tiling mode. It allows setting up buffers that comply to the hardware
> > alignment requirements, by aligning the stride and height to 32 bytes.
> > 
> > Only YUV semiplanar and planar formats are allowed by the ioctl, as the
> > hardware does not support the tiling mode for other formats.
> 
> What's the general feeling about a more generic version of this ioctl?
> There doesn't seem to be anything Allwinner-specific in the ioctl
> arguments.

That's a great suggestion, I am totally in favour of making a generic
fashion of this!

It would also remove the need for a new header dedicated to the sun4i-
drm driver, which was really only motivated by the need for this ioctl.

> It effectively boils down to:
> 
> 	size = driver->get_fb_size_with_modifier(...);
>  	cma_obj = drm_gem_cma_create(drm, size);
> 
> It would look exactly the same for Mali-DP, and probably others too.
> Is it better to try and define something we can share instead of Arm
> adding another nearly identical ioctl() later?
> 
> I think the minimal viable thing would be to just add modifiers to
> your structure (I put them first because padding):
> 
>   struct drm_gem_create_with_modifiers {
>   	__u64 modifiers[4];
>   	__u32 height;
>   	__u32 width;
>   	__u32 format;
>   	/* handle, offsets, pitches, size will be returned */
>   	__u32 handle;
>   	__u32 pitches[4];
>   	__u32 offsets[4];
>   	__u64 size;
>   };

That would totally work for me and our usecase.

Cheers,

Paul

> Thanks,
> -Brian
> 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> > drivers/gpu/drm/sun4i/sun4i_drv.c | 89 +++++++++++++++++++++++++++++++
> > include/uapi/drm/sun4i_drm.h      | 42 +++++++++++++++
> > 2 files changed, 131 insertions(+)
> > create mode 100644 include/uapi/drm/sun4i_drm.h
> > 
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
> > index ccdeae6299eb..5ae32973cf34 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> > @@ -21,6 +21,7 @@
> > #include <drm/drm_gem_cma_helper.h>
> > #include <drm/drm_fb_helper.h>
> > #include <drm/drm_of.h>
> > +#include <drm/sun4i_drm.h>
> > 
> > #include "sun4i_drv.h"
> > #include "sun4i_frontend.h"
> > @@ -28,6 +29,92 @@
> > #include "sun4i_tcon.h"
> > #include "sun8i_tcon_top.h"
> > 
> > +int drm_sun4i_gem_create_tiled(struct drm_device *drm, void *data,
> > +			       struct drm_file *file_priv)
> > +{
> > +	struct drm_sun4i_gem_create_tiled *args = data;
> > +	struct drm_gem_cma_object *cma_obj;
> > +	struct drm_gem_object *gem_obj;
> > +	uint32_t luma_stride, chroma_stride;
> > +	uint32_t luma_height, chroma_height;
> > +	uint32_t chroma_width;
> > +	const struct drm_format_info *info;
> > +	int ret;
> > +
> > +	info = drm_format_info(args->format);
> > +	if (!info)
> > +		return -EINVAL;
> > +
> > +	/* The tiled output format only applies to non-packed YUV formats. */
> > +	if (!info->is_yuv || info->num_planes == 1)
> > +		return -EINVAL;
> > +
> > +	memset(args->pitches, 0, sizeof(args->pitches));
> > +	memset(args->offsets, 0, sizeof(args->offsets));
> > +
> > +	/* Stride and height are aligned to 32 bytes for our tiled format. */
> > +	luma_stride = ALIGN(args->width, 32);
> > +	luma_height = ALIGN(args->height, 32);
> > +
> > +	chroma_width = args->width;
> > +
> > +	/* Semiplanar formats have both U and V data in their chroma plane. */
> > +	if (drm_format_info_is_yuv_semiplanar(info))
> > +		chroma_width *= 2;
> > +
> > +	chroma_stride = ALIGN(DIV_ROUND_UP(chroma_width, info->hsub), 32);
> > +	chroma_height = ALIGN(DIV_ROUND_UP(args->height, info->vsub), 32);
> > +
> > +	if (drm_format_info_is_yuv_semiplanar(info)) {
> > +		args->pitches[0] = luma_stride;
> > +		args->pitches[1] = chroma_stride;
> > +
> > +		args->offsets[0] = 0;
> > +		args->offsets[1] = luma_stride * luma_height;
> > +
> > +		args->size = luma_stride * luma_height +
> > +			     chroma_stride * chroma_height;
> > +	} else if (drm_format_info_is_yuv_planar(info)) {
> > +		args->pitches[0] = luma_stride;
> > +		args->pitches[1] = chroma_stride;
> > +		args->pitches[2] = chroma_stride;
> > +
> > +		args->offsets[0] = 0;
> > +		args->offsets[1] = luma_stride * luma_height;
> > +		args->offsets[2] = luma_stride * luma_height +
> > +				   chroma_stride * chroma_height;
> > +
> > +		args->size = luma_stride * luma_height +
> > +			     chroma_stride * chroma_height * 2;
> > +	} else {
> > +		/* No support for other formats in tiled mode. */
> > +		return -EINVAL;
> > +	}
> > +
> > +	cma_obj = drm_gem_cma_create(drm, args->size);
> > +	if (IS_ERR(cma_obj))
> > +		return PTR_ERR(cma_obj);
> > +
> > +	gem_obj = &cma_obj->base;
> > +
> > +	/*
> > +	 * allocate a id of idr table where the obj is registered
> > +	 * and handle has the id what user can see.
> > +	 */
> > +	ret = drm_gem_handle_create(file_priv, gem_obj, &args->handle);
> > +	/* drop reference from allocate - handle holds it now. */
> > +	drm_gem_object_put_unlocked(gem_obj);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return PTR_ERR_OR_ZERO(cma_obj);
> > +}
> > +
> > +static const struct drm_ioctl_desc sun4i_drv_ioctls[] = {
> > +	DRM_IOCTL_DEF_DRV(SUN4I_GEM_CREATE_TILED, drm_sun4i_gem_create_tiled,
> > +			  DRM_UNLOCKED),
> > +};
> > +
> > static int drm_sun4i_gem_dumb_create(struct drm_file *file_priv,
> > 				     struct drm_device *drm,
> > 				     struct drm_mode_create_dumb *args)
> > @@ -44,6 +131,8 @@ static struct drm_driver sun4i_drv_driver = {
> > 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC,
> > 
> > 	/* Generic Operations */
> > +	.ioctls			= sun4i_drv_ioctls,
> > +	.num_ioctls		= ARRAY_SIZE(sun4i_drv_ioctls),
> > 	.fops			= &sun4i_drv_fops,
> > 	.name			= "sun4i-drm",
> > 	.desc			= "Allwinner sun4i Display Engine",
> > diff --git a/include/uapi/drm/sun4i_drm.h b/include/uapi/drm/sun4i_drm.h
> > new file mode 100644
> > index 000000000000..2c77584b057b
> > --- /dev/null
> > +++ b/include/uapi/drm/sun4i_drm.h
> > @@ -0,0 +1,42 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> > +/* sun4i_drm.h
> > + *
> > + * Copyright (C) 2018 Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > + *
> > + * This program is free software; you can redistribute  it and/or modify it
> > + * under  the terms of  the GNU General  Public License as published by the
> > + * Free Software Foundation;  either version 2 of the  License, or (at your
> > + * option) any later version.
> > + */
> > +
> > +#ifndef _UAPI_SUN4I_DRM_H_
> > +#define _UAPI_SUN4I_DRM_H_
> > +
> > +#include "drm.h"
> > +
> > +#if defined(__cplusplus)
> > +extern "C" {
> > +#endif
> > +
> > +struct drm_sun4i_gem_create_tiled {
> > +	__u32 height;
> > +	__u32 width;
> > +	__u32 format;
> > +	/* handle, offsets, pitches, size will be returned */
> > +	__u32 handle;
> > +	__u32 pitches[4];
> > +	__u32 offsets[4];
> > +	__u64 size;
> > +};
> > +
> > +#define DRM_SUN4I_GEM_CREATE_TILED	0x00
> > +
> > +#define DRM_IOCTL_SUN4I_GEM_CREATE_TILED \
> > +	DRM_IOWR(DRM_COMMAND_BASE + DRM_SUN4I_GEM_CREATE_TILED, \
> > +		 struct drm_sun4i_gem_create_tiled)
> > +
> > +#if defined(__cplusplus)
> > +}
> > +#endif
> > +
> > +#endif /* _UAPI_SUN4I_DRM_H_ */
> > -- 
> > 2.19.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 31/43] drm/sun4i: Add a dedicated ioctl call for allocating tiled buffers

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

On Fri, Nov 23, 2018 at 11:30:20AM +0000, Brian Starkey wrote:
> Hi Paul,
> 
> On Fri, Nov 23, 2018 at 10:25:03AM +0100, Paul Kocialkowski wrote:
> >This introduces a dedicated ioctl for allocating buffers for the VPU
> >tiling mode. It allows setting up buffers that comply to the hardware
> >alignment requirements, by aligning the stride and height to 32 bytes.
> >
> >Only YUV semiplanar and planar formats are allowed by the ioctl, as the
> >hardware does not support the tiling mode for other formats.
> 
> What's the general feeling about a more generic version of this ioctl?
> There doesn't seem to be anything Allwinner-specific in the ioctl
> arguments.

I'm definitely in favor of having such an ioctl.

maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 32/43] drm/sun4i: Pass modifier to backend and frontend format support helpers

[Paul Kocialkowski]

To prepare the introduction of tiled mode support, pass the framebuffer
format modifier to the helpers dealing with format support.

Since only linear mode is supported for now, add corresponding checks in
each helper.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_backend.c  | 10 +++++++---
 drivers/gpu/drm/sun4i/sun4i_backend.h  |  2 +-
 drivers/gpu/drm/sun4i/sun4i_frontend.c |  5 ++++-
 drivers/gpu/drm/sun4i/sun4i_frontend.h |  2 +-
 4 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 986bdb64521c..bbadc2ab9f4d 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -175,10 +175,13 @@ static const uint32_t sun4i_backend_formats[] = {
 	DRM_FORMAT_YVYU,
 };
 
-bool sun4i_backend_format_is_supported(uint32_t fmt)
+bool sun4i_backend_format_is_supported(uint32_t fmt, uint64_t modifier)
 {
 	unsigned int i;
 
+	if (modifier != DRM_FORMAT_MOD_LINEAR)
+		return false;
+
 	for (i = 0; i < ARRAY_SIZE(sun4i_backend_formats); i++)
 		if (sun4i_backend_formats[i] == fmt)
 			return true;
@@ -455,14 +458,15 @@ static bool sun4i_backend_plane_uses_frontend(struct drm_plane_state *state)
 	struct sun4i_layer *layer = plane_to_sun4i_layer(state->plane);
 	struct sun4i_backend *backend = layer->backend;
 	uint32_t format = state->fb->format->format;
+	uint64_t modifier = state->fb->modifier;
 
 	if (IS_ERR(backend->frontend))
 		return false;
 
-	if (!sun4i_frontend_format_is_supported(format))
+	if (!sun4i_frontend_format_is_supported(format, modifier))
 		return false;
 
-	if (!sun4i_backend_format_is_supported(format))
+	if (!sun4i_backend_format_is_supported(format, modifier))
 		return true;
 
 	/*
diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
index 93db3af675b3..01f66463271b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
@@ -198,7 +198,7 @@ engine_to_sun4i_backend(struct sunxi_engine *engine)
 
 void sun4i_backend_layer_enable(struct sun4i_backend *backend,
 				int layer, bool enable);
-bool sun4i_backend_format_is_supported(uint32_t fmt);
+bool sun4i_backend_format_is_supported(uint32_t fmt, uint64_t modifier);
 int sun4i_backend_update_layer_coord(struct sun4i_backend *backend,
 				     int layer, struct drm_plane *plane);
 int sun4i_backend_update_layer_formats(struct sun4i_backend *backend,
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index 001139417cf5..b78e0a208c95 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -299,10 +299,13 @@ static const uint32_t sun4i_frontend_formats[] = {
 	DRM_FORMAT_YVYU,
 };
 
-bool sun4i_frontend_format_is_supported(uint32_t fmt)
+bool sun4i_frontend_format_is_supported(uint32_t fmt, uint64_t modifier)
 {
 	unsigned int i;
 
+	if (modifier != DRM_FORMAT_MOD_LINEAR)
+		return false;
+
 	for (i = 0; i < ARRAY_SIZE(sun4i_frontend_formats); i++)
 		if (sun4i_frontend_formats[i] == fmt)
 			return true;
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.h b/drivers/gpu/drm/sun4i/sun4i_frontend.h
index 54ec71a3e2f4..b209eacdf8c2 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.h
@@ -115,6 +115,6 @@ void sun4i_frontend_update_coord(struct sun4i_frontend *frontend,
 				 struct drm_plane *plane);
 int sun4i_frontend_update_formats(struct sun4i_frontend *frontend,
 				  struct drm_plane *plane, uint32_t out_fmt);
-bool sun4i_frontend_format_is_supported(uint32_t fmt);
+bool sun4i_frontend_format_is_supported(uint32_t fmt, uint64_t modifier);
 
 #endif /* _SUN4I_FRONTEND_H_ */
-- 
2.19.1

Re: [PATCH v2 32/43] drm/sun4i: Pass modifier to backend and frontend format support helpers

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 493 bytes --]

On Fri, Nov 23, 2018 at 10:25:04AM +0100, Paul Kocialkowski wrote:
> To prepare the introduction of tiled mode support, pass the framebuffer
> format modifier to the helpers dealing with format support.
> 
> Since only linear mode is supported for now, add corresponding checks in
> each helper.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 33/43] drm/sun4i: frontend: Add support for tiled YUV input mode configuration

[Paul Kocialkowski]

This introduces the data input mode definitions for the tiled YUV mode,
that are used in the input mode helper if tiling is requested.

The modifier is passed to the helper from the framebuffer to determine
if tiling is requested.

Only semiplanar and planar YUV formats are supported for tiling mode.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 14 ++++++++++----
 drivers/gpu/drm/sun4i/sun4i_frontend.h |  2 ++
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index b78e0a208c95..efa1ff0802bd 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -186,9 +186,11 @@ static int sun4i_frontend_drm_format_to_input_fmt(uint32_t fmt, u32 *val)
 	return 0;
 }
 
-static int sun4i_frontend_drm_format_to_input_mode(uint32_t fmt, u32 *val)
+static int sun4i_frontend_drm_format_to_input_mode(uint32_t fmt,
+						   uint64_t modifier, u32 *val)
 {
 	int num_planes = drm_format_num_planes(fmt);
+	bool tiled = (modifier == DRM_FORMAT_MOD_ALLWINNER_TILED);
 
 	switch (num_planes) {
 	case 1:
@@ -196,11 +198,13 @@ static int sun4i_frontend_drm_format_to_input_mode(uint32_t fmt, u32 *val)
 		return 0;
 
 	case 2:
-		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_SEMIPLANAR;
+		*val = tiled ? SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_MB32_SEMIPLANAR
+			     : SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_SEMIPLANAR;
 		return 0;
 
 	case 3:
-		*val = SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_PLANAR;
+		*val = tiled ? SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_MB32_PLANAR
+			     : SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_PLANAR;
 		return 0;
 
 	default:
@@ -320,6 +324,7 @@ int sun4i_frontend_update_formats(struct sun4i_frontend *frontend,
 	struct drm_plane_state *state = plane->state;
 	struct drm_framebuffer *fb = state->fb;
 	uint32_t format = fb->format->format;
+	uint64_t modifier = fb->modifier;
 	u32 out_fmt_val;
 	u32 in_fmt_val, in_mod_val, in_ps_val;
 	unsigned int i;
@@ -332,7 +337,8 @@ int sun4i_frontend_update_formats(struct sun4i_frontend *frontend,
 		return ret;
 	}
 
-	ret = sun4i_frontend_drm_format_to_input_mode(format, &in_mod_val);
+	ret = sun4i_frontend_drm_format_to_input_mode(format, modifier,
+						      &in_mod_val);
 	if (ret) {
 		DRM_DEBUG_DRIVER("Invalid input mode\n");
 		return ret;
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.h b/drivers/gpu/drm/sun4i/sun4i_frontend.h
index b209eacdf8c2..ae53ee644694 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.h
@@ -33,6 +33,8 @@
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_PLANAR	(0 << 8)
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_PACKED	(1 << 8)
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_SEMIPLANAR	(2 << 8)
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_MB32_PLANAR	(4 << 8)
+#define SUN4I_FRONTEND_INPUT_FMT_DATA_MOD_MB32_SEMIPLANAR (6 << 8)
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV444	(0 << 4)
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV422	(1 << 4)
 #define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT_YUV420	(2 << 4)
-- 
2.19.1

[PATCH v2 34/43] drm/sun4i: Add buffer stride and offset configuration for tiling mode

[Paul Kocialkowski]

This introduces stride and offset configuration for the VPU tiling mode.
Stride is calculated differently than it is for linear formats and an
offset is calculated, for which new register definitions are introduced.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 54 ++++++++++++++++++++++++--
 drivers/gpu/drm/sun4i/sun4i_frontend.h |  7 ++++
 2 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index efa1ff0802bd..3f76a5572449 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -125,21 +125,69 @@ void sun4i_frontend_update_buffer(struct sun4i_frontend *frontend,
 {
 	struct drm_plane_state *state = plane->state;
 	struct drm_framebuffer *fb = state->fb;
+	unsigned int strides[3] = {};
+
 	dma_addr_t paddr;
 	bool swap;
 
+	if (fb->modifier == DRM_FORMAT_MOD_ALLWINNER_TILED) {
+		unsigned int width = state->src_w >> 16;
+		unsigned int offset;
+
+		/*
+		 * In MB32 tiled mode, the stride is defined as the distance
+		 * between the start of the end line of the current tile and
+		 * the start of the first line in the next vertical tile.
+		 *
+		 * Tiles are represented in row-major order, thus the end line
+		 * of current tile starts at: 31 * 32 (31 lines of 32 cols),
+		 * the next vertical tile starts at: 32-bit-aligned-width * 32
+		 * and the distance is: 32 * (32-bit-aligned-width - 31).
+		 */
+
+		strides[0] = (fb->pitches[0] - 31) * 32;
+
+		/* Offset of the bottom-right point in the end tile. */
+		offset = (width + (32 - 1)) & (32 - 1);
+
+		regmap_write(frontend->regs, SUN4I_FRONTEND_TB_OFF0_REG,
+			     SUN4I_FRONTEND_TB_OFF_X1(offset));
+
+		if (fb->format->num_planes > 1) {
+			strides[1] = (fb->pitches[1] - 31) * 32;
+
+			regmap_write(frontend->regs, SUN4I_FRONTEND_TB_OFF1_REG,
+				     SUN4I_FRONTEND_TB_OFF_X1(offset));
+		}
+
+		if (fb->format->num_planes > 2) {
+			strides[2] = (fb->pitches[2] - 31) * 32;
+
+			regmap_write(frontend->regs, SUN4I_FRONTEND_TB_OFF2_REG,
+				     SUN4I_FRONTEND_TB_OFF_X1(offset));
+		}
+	} else {
+		strides[0] = fb->pitches[0];
+
+		if (fb->format->num_planes > 1)
+			strides[1] = fb->pitches[1];
+
+		if (fb->format->num_planes > 2)
+			strides[2] = fb->pitches[2];
+	}
+
 	/* Set the line width */
 	DRM_DEBUG_DRIVER("Frontend stride: %d bytes\n", fb->pitches[0]);
 	regmap_write(frontend->regs, SUN4I_FRONTEND_LINESTRD0_REG,
-		     fb->pitches[0]);
+		     strides[0]);
 
 	if (fb->format->num_planes > 1)
 		regmap_write(frontend->regs, SUN4I_FRONTEND_LINESTRD1_REG,
-			     fb->pitches[1]);
+			     strides[1]);
 
 	if (fb->format->num_planes > 2)
 		regmap_write(frontend->regs, SUN4I_FRONTEND_LINESTRD2_REG,
-			     fb->pitches[2]);
+			     strides[2]);
 
 	/* Some planar formats require chroma channel swapping by hand. */
 	swap = sun4i_frontend_format_chroma_requires_swap(fb->format->format);
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.h b/drivers/gpu/drm/sun4i/sun4i_frontend.h
index ae53ee644694..822d39ca4b8e 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.h
@@ -25,6 +25,13 @@
 #define SUN4I_FRONTEND_BUF_ADDR1_REG		0x024
 #define SUN4I_FRONTEND_BUF_ADDR2_REG		0x028
 
+#define SUN4I_FRONTEND_TB_OFF0_REG		0x030
+#define SUN4I_FRONTEND_TB_OFF1_REG		0x034
+#define SUN4I_FRONTEND_TB_OFF2_REG		0x038
+#define SUN4I_FRONTEND_TB_OFF_X1(x1)			((x1) << 16)
+#define SUN4I_FRONTEND_TB_OFF_Y0(y0)			((y0) << 8)
+#define SUN4I_FRONTEND_TB_OFF_X0(x0)			(x0)
+
 #define SUN4I_FRONTEND_LINESTRD0_REG		0x040
 #define SUN4I_FRONTEND_LINESTRD1_REG		0x044
 #define SUN4I_FRONTEND_LINESTRD2_REG		0x048
-- 
2.19.1

Re: [PATCH v2 34/43] drm/sun4i: Add buffer stride and offset configuration for tiling mode

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2816 bytes --]

On Fri, Nov 23, 2018 at 10:25:06AM +0100, Paul Kocialkowski wrote:
> This introduces stride and offset configuration for the VPU tiling mode.
> Stride is calculated differently than it is for linear formats and an
> offset is calculated, for which new register definitions are introduced.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  drivers/gpu/drm/sun4i/sun4i_frontend.c | 54 ++++++++++++++++++++++++--
>  drivers/gpu/drm/sun4i/sun4i_frontend.h |  7 ++++
>  2 files changed, 58 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
> index efa1ff0802bd..3f76a5572449 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
> @@ -125,21 +125,69 @@ void sun4i_frontend_update_buffer(struct sun4i_frontend *frontend,
>  {
>  	struct drm_plane_state *state = plane->state;
>  	struct drm_framebuffer *fb = state->fb;
> +	unsigned int strides[3] = {};
> +
>  	dma_addr_t paddr;
>  	bool swap;
>  
> +	if (fb->modifier == DRM_FORMAT_MOD_ALLWINNER_TILED) {
> +		unsigned int width = state->src_w >> 16;
> +		unsigned int offset;
> +
> +		/*
> +		 * In MB32 tiled mode, the stride is defined as the distance
> +		 * between the start of the end line of the current tile and
> +		 * the start of the first line in the next vertical tile.
> +		 *
> +		 * Tiles are represented in row-major order, thus the end line
> +		 * of current tile starts at: 31 * 32 (31 lines of 32 cols),
> +		 * the next vertical tile starts at: 32-bit-aligned-width * 32
> +		 * and the distance is: 32 * (32-bit-aligned-width - 31).
> +		 */
> +
> +		strides[0] = (fb->pitches[0] - 31) * 32;
> +
> +		/* Offset of the bottom-right point in the end tile. */
> +		offset = (width + (32 - 1)) & (32 - 1);

Those computations are a bit obscure. I guess adding a bunch of
defines, and using the round_up / _down and ALIGN macros would help

> +		regmap_write(frontend->regs, SUN4I_FRONTEND_TB_OFF0_REG,
> +			     SUN4I_FRONTEND_TB_OFF_X1(offset));
> +
> +		if (fb->format->num_planes > 1) {
> +			strides[1] = (fb->pitches[1] - 31) * 32;
> +
> +			regmap_write(frontend->regs, SUN4I_FRONTEND_TB_OFF1_REG,
> +				     SUN4I_FRONTEND_TB_OFF_X1(offset));
> +		}
> +
> +		if (fb->format->num_planes > 2) {
> +			strides[2] = (fb->pitches[2] - 31) * 32;
> +
> +			regmap_write(frontend->regs, SUN4I_FRONTEND_TB_OFF2_REG,
> +				     SUN4I_FRONTEND_TB_OFF_X1(offset));
> +		}

I guess we could fall in a situation where this is not cleared when
moving from a format with 3 planes to one with 2 for example. Would
that break anything?

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 34/43] drm/sun4i: Add buffer stride and offset configuration for tiling mode

[Paul Kocialkowski]

Hi,

On Tue, 2018-11-27 at 10:24 +0100, Maxime Ripard wrote:
> On Fri, Nov 23, 2018 at 10:25:06AM +0100, Paul Kocialkowski wrote:
> > This introduces stride and offset configuration for the VPU tiling mode.
> > Stride is calculated differently than it is for linear formats and an
> > offset is calculated, for which new register definitions are introduced.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  drivers/gpu/drm/sun4i/sun4i_frontend.c | 54 ++++++++++++++++++++++++--
> >  drivers/gpu/drm/sun4i/sun4i_frontend.h |  7 ++++
> >  2 files changed, 58 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
> > index efa1ff0802bd..3f76a5572449 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
> > @@ -125,21 +125,69 @@ void sun4i_frontend_update_buffer(struct sun4i_frontend *frontend,
> >  {
> >  	struct drm_plane_state *state = plane->state;
> >  	struct drm_framebuffer *fb = state->fb;
> > +	unsigned int strides[3] = {};
> > +
> >  	dma_addr_t paddr;
> >  	bool swap;
> >  
> > +	if (fb->modifier == DRM_FORMAT_MOD_ALLWINNER_TILED) {
> > +		unsigned int width = state->src_w >> 16;
> > +		unsigned int offset;
> > +
> > +		/*
> > +		 * In MB32 tiled mode, the stride is defined as the distance
> > +		 * between the start of the end line of the current tile and
> > +		 * the start of the first line in the next vertical tile.
> > +		 *
> > +		 * Tiles are represented in row-major order, thus the end line
> > +		 * of current tile starts at: 31 * 32 (31 lines of 32 cols),
> > +		 * the next vertical tile starts at: 32-bit-aligned-width * 32
> > +		 * and the distance is: 32 * (32-bit-aligned-width - 31).
> > +		 */
> > +
> > +		strides[0] = (fb->pitches[0] - 31) * 32;
> > +
> > +		/* Offset of the bottom-right point in the end tile. */
> > +		offset = (width + (32 - 1)) & (32 - 1);
> 
> Those computations are a bit obscure. I guess adding a bunch of
> defines, and using the round_up / _down and ALIGN macros would help

Fair enough, I will add a more explicit comment and a macro in v3.

> > +		regmap_write(frontend->regs, SUN4I_FRONTEND_TB_OFF0_REG,
> > +			     SUN4I_FRONTEND_TB_OFF_X1(offset));
> > +
> > +		if (fb->format->num_planes > 1) {
> > +			strides[1] = (fb->pitches[1] - 31) * 32;
> > +
> > +			regmap_write(frontend->regs, SUN4I_FRONTEND_TB_OFF1_REG,
> > +				     SUN4I_FRONTEND_TB_OFF_X1(offset));
> > +		}
> > +
> > +		if (fb->format->num_planes > 2) {
> > +			strides[2] = (fb->pitches[2] - 31) * 32;
> > +
> > +			regmap_write(frontend->regs, SUN4I_FRONTEND_TB_OFF2_REG,
> > +				     SUN4I_FRONTEND_TB_OFF_X1(offset));
> > +		}
> 
> I guess we could fall in a situation where this is not cleared when
> moving from a format with 3 planes to one with 2 for example. Would
> that break anything?

I have tested this case and nothing breaks so we're good!

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

[PATCH v2 35/43] drm/sun4i: frontend: Add and use helper for checking tiling support

[Paul Kocialkowski]

This introduces a helper to check whether a frontend input format
supports tiling mode. This helper is used when tiling is requested in
the frontend format support helper.

Only semiplanar and planar YUV formats are supported by the hardware.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index 3f76a5572449..b9c18fa2fff3 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -120,6 +120,26 @@ static bool sun4i_frontend_format_chroma_requires_swap(uint32_t fmt)
 	}
 }
 
+static bool sun4i_frontend_format_supports_tiling(uint32_t fmt)
+{
+	switch (fmt) {
+	case DRM_FORMAT_NV12:
+	case DRM_FORMAT_NV16:
+	case DRM_FORMAT_NV21:
+	case DRM_FORMAT_NV61:
+	case DRM_FORMAT_YUV411:
+	case DRM_FORMAT_YUV420:
+	case DRM_FORMAT_YUV422:
+	case DRM_FORMAT_YVU420:
+	case DRM_FORMAT_YVU422:
+	case DRM_FORMAT_YVU411:
+		return true;
+
+	default:
+		return false;
+	}
+}
+
 void sun4i_frontend_update_buffer(struct sun4i_frontend *frontend,
 				  struct drm_plane *plane)
 {
@@ -355,7 +375,9 @@ bool sun4i_frontend_format_is_supported(uint32_t fmt, uint64_t modifier)
 {
 	unsigned int i;
 
-	if (modifier != DRM_FORMAT_MOD_LINEAR)
+	if (modifier == DRM_FORMAT_MOD_ALLWINNER_TILED)
+		return sun4i_frontend_format_supports_tiling(fmt);
+	else if (modifier != DRM_FORMAT_MOD_LINEAR)
 		return false;
 
 	for (i = 0; i < ARRAY_SIZE(sun4i_frontend_formats); i++)
-- 
2.19.1

[PATCH v2 36/43] drm/sun4i: layer: Add tiled modifier support and helper

[Paul Kocialkowski]

From: Paul Kocialkowski <contact@paulk.fr>

This introduces a list of supported modifiers for the driver, that
includes the Allwinner tiled modifier, as well as a format_mod_supported
callback.

The callback uses both the backend and frontend helpers to indicate
per-format modifier support (including for the linear modifier).

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/gpu/drm/sun4i/sun4i_layer.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index 2fb27819ad20..a828572acdbb 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -113,6 +113,13 @@ static void sun4i_backend_layer_atomic_update(struct drm_plane *plane,
 	sun4i_backend_layer_enable(backend, layer->id, true);
 }
 
+static bool sun4i_layer_format_mod_supported(struct drm_plane *plane,
+					     uint32_t format, uint64_t modifier)
+{
+	return sun4i_backend_format_is_supported(format, modifier) ||
+	       sun4i_frontend_format_is_supported(format, modifier);
+}
+
 static const struct drm_plane_helper_funcs sun4i_backend_layer_helper_funcs = {
 	.atomic_disable	= sun4i_backend_layer_atomic_disable,
 	.atomic_update	= sun4i_backend_layer_atomic_update,
@@ -125,6 +132,7 @@ static const struct drm_plane_funcs sun4i_backend_layer_funcs = {
 	.disable_plane		= drm_atomic_helper_disable_plane,
 	.reset			= sun4i_backend_layer_reset,
 	.update_plane		= drm_atomic_helper_update_plane,
+	.format_mod_supported	= sun4i_layer_format_mod_supported,
 };
 
 static const uint32_t sun4i_layer_formats[] = {
@@ -155,6 +163,12 @@ static const uint32_t sun4i_layer_formats[] = {
 	DRM_FORMAT_YVYU,
 };
 
+static const uint64_t sun4i_layer_modifiers[] = {
+	DRM_FORMAT_MOD_LINEAR,
+	DRM_FORMAT_MOD_ALLWINNER_TILED,
+	DRM_FORMAT_MOD_INVALID
+};
+
 static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
 						struct sun4i_backend *backend,
 						enum drm_plane_type type)
@@ -171,7 +185,7 @@ static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
 				       &sun4i_backend_layer_funcs,
 				       sun4i_layer_formats,
 				       ARRAY_SIZE(sun4i_layer_formats),
-				       NULL, type, NULL);
+				       sun4i_layer_modifiers, type, NULL);
 	if (ret) {
 		dev_err(drm->dev, "Couldn't initialize layer\n");
 		return ERR_PTR(ret);
-- 
2.19.1

[PATCH v2 37/43] drm/sun4i: drv: Allow framebuffer modifiers in mode config

[Paul Kocialkowski]

From: Paul Kocialkowski <contact@paulk.fr>

This is the final step to indicate to the core that our driver
supports framebuffer modifiers.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/gpu/drm/sun4i/sun4i_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
index 5ae32973cf34..a041febe391a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -186,6 +186,7 @@ static int sun4i_drv_bind(struct device *dev)
 	}
 
 	drm_mode_config_init(drm);
+	drm->mode_config.allow_fb_modifiers = true;
 
 	ret = component_bind_all(drm->dev, drm);
 	if (ret) {
-- 
2.19.1

[PATCH v2 38/43] drm/sun4i: Move access control before setting the register as documented

[Paul Kocialkowski]

From: Maxime Ripard <maxime.ripard@bootlin.com>

Unlike what is currently being done, the ACCESS_CTRL bit documentation asks
that this bit should be set before modifying any register. The code in the
BSP also does this, so make sure we do this as well.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index b9c18fa2fff3..2c9dcb4ffde0 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -74,6 +74,10 @@ static void sun4i_frontend_scaler_init(struct sun4i_frontend *frontend)
 {
 	int i;
 
+	regmap_write_bits(frontend->regs, SUN4I_FRONTEND_FRM_CTRL_REG,
+			  SUN4I_FRONTEND_FRM_CTRL_COEF_ACCESS_CTRL,
+			  SUN4I_FRONTEND_FRM_CTRL_COEF_ACCESS_CTRL);
+
 	for (i = 0; i < 32; i++) {
 		regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_HORZCOEF0_REG(i),
 			     sun4i_frontend_horz_coef[2 * i]);
@@ -89,9 +93,6 @@ static void sun4i_frontend_scaler_init(struct sun4i_frontend *frontend)
 			     sun4i_frontend_vert_coef[i]);
 	}
 
-	regmap_update_bits(frontend->regs, SUN4I_FRONTEND_FRM_CTRL_REG,
-			   SUN4I_FRONTEND_FRM_CTRL_COEF_ACCESS_CTRL,
-			   SUN4I_FRONTEND_FRM_CTRL_COEF_ACCESS_CTRL);
 }
 
 int sun4i_frontend_init(struct sun4i_frontend *frontend)
-- 
2.19.1

Re: [PATCH v2 38/43] drm/sun4i: Move access control before setting the register as documented

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

On Fri, Nov 23, 2018 at 10:25:10AM +0100, Paul Kocialkowski wrote:
> From: Maxime Ripard <maxime.ripard@bootlin.com>
> 
> Unlike what is currently being done, the ACCESS_CTRL bit documentation asks
> that this bit should be set before modifying any register. The code in the
> BSP also does this, so make sure we do this as well.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>

This patch (and the subsequent ones) are missing your SoB.

Maxime
-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 39/43] drm/sun4i: frontend: Add a quirk structure

[Paul Kocialkowski]

From: Maxime Ripard <maxime.ripard@bootlin.com>

The ACCESS_CTRL bit is not found on all the variants of the frontend, so
let's introduce a structure that will hold whether or not we need to set
it, and associate it with the compatible.

This will be extended for further similar quirks later on.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 21 +++++++++++++++++----
 drivers/gpu/drm/sun4i/sun4i_frontend.h |  6 ++++++
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index 2c9dcb4ffde0..368391a9e5fd 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -10,6 +10,7 @@
 #include <linux/clk.h>
 #include <linux/component.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
@@ -74,9 +75,10 @@ static void sun4i_frontend_scaler_init(struct sun4i_frontend *frontend)
 {
 	int i;
 
-	regmap_write_bits(frontend->regs, SUN4I_FRONTEND_FRM_CTRL_REG,
-			  SUN4I_FRONTEND_FRM_CTRL_COEF_ACCESS_CTRL,
-			  SUN4I_FRONTEND_FRM_CTRL_COEF_ACCESS_CTRL);
+	if (frontend->data->has_coef_access_ctrl)
+		regmap_write_bits(frontend->regs, SUN4I_FRONTEND_FRM_CTRL_REG,
+				  SUN4I_FRONTEND_FRM_CTRL_COEF_ACCESS_CTRL,
+				  SUN4I_FRONTEND_FRM_CTRL_COEF_ACCESS_CTRL);
 
 	for (i = 0; i < 32; i++) {
 		regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_HORZCOEF0_REG(i),
@@ -547,6 +549,10 @@ static int sun4i_frontend_bind(struct device *dev, struct device *master,
 	frontend->dev = dev;
 	frontend->node = dev->of_node;
 
+	frontend->data = of_device_get_match_data(dev);
+	if (!frontend->data)
+		return -ENODEV;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	regs = devm_ioremap_resource(dev, res);
 	if (IS_ERR(regs))
@@ -659,8 +665,15 @@ static const struct dev_pm_ops sun4i_frontend_pm_ops = {
 	.runtime_suspend	= sun4i_frontend_runtime_suspend,
 };
 
+static const struct sun4i_frontend_data sun8i_a33_frontend = {
+	.has_coef_access_ctrl	= true,
+};
+
 const struct of_device_id sun4i_frontend_of_table[] = {
-	{ .compatible = "allwinner,sun8i-a33-display-frontend" },
+	{
+		.compatible = "allwinner,sun8i-a33-display-frontend",
+		.data = &sun8i_a33_frontend
+	},
 	{ }
 };
 EXPORT_SYMBOL(sun4i_frontend_of_table);
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.h b/drivers/gpu/drm/sun4i/sun4i_frontend.h
index 822d39ca4b8e..daf3fa5d01ba 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.h
@@ -100,6 +100,10 @@ struct drm_plane;
 struct regmap;
 struct reset_control;
 
+struct sun4i_frontend_data {
+	bool	has_coef_access_ctrl;
+};
+
 struct sun4i_frontend {
 	struct list_head	list;
 	struct device		*dev;
@@ -110,6 +114,8 @@ struct sun4i_frontend {
 	struct clk		*ram_clk;
 	struct regmap		*regs;
 	struct reset_control	*reset;
+
+	const struct sun4i_frontend_data	*data;
 };
 
 extern const struct of_device_id sun4i_frontend_of_table[];
-- 
2.19.1

[PATCH v2 40/43] drm/sun4i: Set the coef_rdy bit right after the coef have been set

[Paul Kocialkowski]

From: Maxime Ripard <maxime.ripard@bootlin.com>

The COEF_RDY bit is used to tell the hardware that new FIR filters
coefficients have been written to the registers and that the hardware
should take them into account starting next frame.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index 368391a9e5fd..0e92f59efd49 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -95,6 +95,10 @@ static void sun4i_frontend_scaler_init(struct sun4i_frontend *frontend)
 			     sun4i_frontend_vert_coef[i]);
 	}
 
+	regmap_write_bits(frontend->regs,
+			  SUN4I_FRONTEND_FRM_CTRL_REG,
+			  SUN4I_FRONTEND_FRM_CTRL_COEF_RDY,
+			  SUN4I_FRONTEND_FRM_CTRL_COEF_RDY);
 }
 
 int sun4i_frontend_init(struct sun4i_frontend *frontend)
-- 
2.19.1

[PATCH v2 41/43] drm/sun4i: Make COEF_RDY conditional

[Paul Kocialkowski]

From: Maxime Ripard <maxime.ripard@bootlin.com>

The COEF_RDY bit isn't found in all the SoCs featuring some variant of the
frontend.

Add it to our quirks structure.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 9 +++++----
 drivers/gpu/drm/sun4i/sun4i_frontend.h | 1 +
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index 0e92f59efd49..f50507ee6d75 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -95,10 +95,11 @@ static void sun4i_frontend_scaler_init(struct sun4i_frontend *frontend)
 			     sun4i_frontend_vert_coef[i]);
 	}
 
-	regmap_write_bits(frontend->regs,
-			  SUN4I_FRONTEND_FRM_CTRL_REG,
-			  SUN4I_FRONTEND_FRM_CTRL_COEF_RDY,
-			  SUN4I_FRONTEND_FRM_CTRL_COEF_RDY);
+	if (frontend->data->has_coef_rdy)
+		regmap_write_bits(frontend->regs,
+				  SUN4I_FRONTEND_FRM_CTRL_REG,
+				  SUN4I_FRONTEND_FRM_CTRL_COEF_RDY,
+				  SUN4I_FRONTEND_FRM_CTRL_COEF_RDY);
 }
 
 int sun4i_frontend_init(struct sun4i_frontend *frontend)
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.h b/drivers/gpu/drm/sun4i/sun4i_frontend.h
index daf3fa5d01ba..3618993f8e49 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.h
@@ -102,6 +102,7 @@ struct reset_control;
 
 struct sun4i_frontend_data {
 	bool	has_coef_access_ctrl;
+	bool	has_coef_rdy;
 };
 
 struct sun4i_frontend {
-- 
2.19.1

[PATCH v2 42/43] drm/sun4i: frontend: Move the FIR filter phases to our quirks

[Paul Kocialkowski]

From: Maxime Ripard <maxime.ripard@bootlin.com>

The FIR filters phase depend on the SoC, so let's move it to our quirks
structure instead of removing them.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 28 ++++++++++++++++++++------
 drivers/gpu/drm/sun4i/sun4i_frontend.h |  5 +++++
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index f50507ee6d75..427c51029a9c 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -438,12 +438,18 @@ int sun4i_frontend_update_formats(struct sun4i_frontend *frontend,
 	 * I have no idea what this does exactly, but it seems to be
 	 * related to the scaler FIR filter phase parameters.
 	 */
-	regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_HORZPHASE_REG, 0x400);
-	regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_HORZPHASE_REG, 0x400);
-	regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_VERTPHASE0_REG, 0x400);
-	regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_VERTPHASE0_REG, 0x400);
-	regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_VERTPHASE1_REG, 0x400);
-	regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_VERTPHASE1_REG, 0x400);
+	regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_HORZPHASE_REG,
+		     frontend->data->ch_phase[0].horzphase);
+	regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_HORZPHASE_REG,
+		     frontend->data->ch_phase[1].horzphase);
+	regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_VERTPHASE0_REG,
+		     frontend->data->ch_phase[0].vertphase[0]);
+	regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_VERTPHASE0_REG,
+		     frontend->data->ch_phase[1].vertphase[0]);
+	regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_VERTPHASE1_REG,
+		     frontend->data->ch_phase[0].vertphase[1]);
+	regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_VERTPHASE1_REG,
+		     frontend->data->ch_phase[1].vertphase[1]);
 
 	if (fb->format->is_yuv && !drm_format_is_yuv(out_fmt)) {
 		/* Setup the CSC engine for YUV to RGB conversion. */
@@ -671,6 +677,16 @@ static const struct dev_pm_ops sun4i_frontend_pm_ops = {
 };
 
 static const struct sun4i_frontend_data sun8i_a33_frontend = {
+	.ch_phase		= {
+		{
+			.horzphase = 0x400,
+			.vertphase = { 0x400, 0x400 },
+		},
+		{
+			.horzphase = 0x400,
+			.vertphase = { 0x400, 0x400 },
+		},
+	},
 	.has_coef_access_ctrl	= true,
 };
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.h b/drivers/gpu/drm/sun4i/sun4i_frontend.h
index 3618993f8e49..45f5f4f3d06f 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.h
@@ -103,6 +103,11 @@ struct reset_control;
 struct sun4i_frontend_data {
 	bool	has_coef_access_ctrl;
 	bool	has_coef_rdy;
+
+	struct {
+		u32	horzphase;
+		u32	vertphase[2];
+	} ch_phase[2];
 };
 
 struct sun4i_frontend {
-- 
2.19.1

[PATCH v2 43/43] drm/sun4i: frontend: Add A20-specific device-tree compatible and quirks

[Paul Kocialkowski]

This adds the appropriate device-tree compatible and quirk data for
hooking frontend support for the A20. It supports the FIR coefficients
ready bit but not the access control bit. It also takes different phase
values than the A33 for these coefficients.

The compatible is already used in the A20 device-tree and already
documented in the device-tree bindings.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_frontend.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
index 427c51029a9c..4a9cb478167f 100644
--- a/drivers/gpu/drm/sun4i/sun4i_frontend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
@@ -676,6 +676,20 @@ static const struct dev_pm_ops sun4i_frontend_pm_ops = {
 	.runtime_suspend	= sun4i_frontend_runtime_suspend,
 };
 
+static const struct sun4i_frontend_data sun7i_a20_frontend = {
+	.ch_phase		= {
+		{
+			.horzphase = 0,
+			.vertphase = { 0, 0 },
+		},
+		{
+			.horzphase = 0xfc000,
+			.vertphase = { 0xfc000, 0xfc000 },
+		},
+	},
+	.has_coef_rdy		= true,
+};
+
 static const struct sun4i_frontend_data sun8i_a33_frontend = {
 	.ch_phase		= {
 		{
@@ -691,6 +705,10 @@ static const struct sun4i_frontend_data sun8i_a33_frontend = {
 };
 
 const struct of_device_id sun4i_frontend_of_table[] = {
+	{
+		.compatible = "allwinner,sun7i-a20-display-frontend",
+		.data = &sun7i_a20_frontend
+	},
 	{
 		.compatible = "allwinner,sun8i-a33-display-frontend",
 		.data = &sun8i_a33_frontend
-- 
2.19.1