[PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Damian Hobson-Garcia]

Add additional pixel formats for which blending is disabling when
DRM_MODE_BLEND_PIXEL_NONE is set.

Refactor the fourcc selection into a separate function to handle the
increased number of formats.

Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
---
 drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
 1 file changed, 32 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
index 45c05d0ffc70..96241c03b60f 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
@@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
 	DRM_FORMAT_Y212,
 };
 
+static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
+{
+	u32 fourcc = state->format->fourcc;
+
+	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
+		switch (fourcc) {
+		case DRM_FORMAT_ARGB1555:
+			fourcc = DRM_FORMAT_XRGB1555;
+			break;
+
+		case DRM_FORMAT_ARGB4444:
+			fourcc = DRM_FORMAT_XRGB4444;
+			break;
+
+		case DRM_FORMAT_ARGB8888:
+			fourcc = DRM_FORMAT_XRGB8888;
+			break;
+
+		case DRM_FORMAT_BGRA8888:
+			fourcc = DRM_FORMAT_BGRX8888;
+			break;
+
+		case DRM_FORMAT_RGBA1010102:
+			fourcc = DRM_FORMAT_RGBX1010102;
+			break;
+		}
+	}
+
+	return fourcc;
+}
+
 static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
 {
 	struct rcar_du_vsp_plane_state *state =
@@ -189,7 +220,7 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
 		.alpha = state->state.alpha >> 8,
 		.zpos = state->state.zpos,
 	};
-	u32 fourcc = state->format->fourcc;
+	u32 fourcc = rcar_du_vsp_state_get_format(state);
 	unsigned int i;
 
 	cfg.src.left = state->state.src.x1 >> 16;
@@ -206,22 +237,6 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
 		cfg.mem[i] = sg_dma_address(state->sg_tables[i].sgl)
 			   + fb->offsets[i];
 
-	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
-		switch (fourcc) {
-		case DRM_FORMAT_ARGB1555:
-			fourcc = DRM_FORMAT_XRGB1555;
-			break;
-
-		case DRM_FORMAT_ARGB4444:
-			fourcc = DRM_FORMAT_XRGB4444;
-			break;
-
-		case DRM_FORMAT_ARGB8888:
-			fourcc = DRM_FORMAT_XRGB8888;
-			break;
-		}
-	}
-
 	format = rcar_du_format_info(fourcc);
 	cfg.pixelformat = format->v4l2;
 
-- 
2.25.1

[PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Damian Hobson-Garcia]

Add additional pixel formats for which blending is disabling when
DRM_MODE_BLEND_PIXEL_NONE is set.

Refactor the fourcc selection into a separate function to handle the
increased number of formats.

Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
---
 drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
 1 file changed, 32 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
index 45c05d0ffc70..96241c03b60f 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
@@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
 	DRM_FORMAT_Y212,
 };
 
+static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
+{
+	u32 fourcc = state->format->fourcc;
+
+	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
+		switch (fourcc) {
+		case DRM_FORMAT_ARGB1555:
+			fourcc = DRM_FORMAT_XRGB1555;
+			break;
+
+		case DRM_FORMAT_ARGB4444:
+			fourcc = DRM_FORMAT_XRGB4444;
+			break;
+
+		case DRM_FORMAT_ARGB8888:
+			fourcc = DRM_FORMAT_XRGB8888;
+			break;
+
+		case DRM_FORMAT_BGRA8888:
+			fourcc = DRM_FORMAT_BGRX8888;
+			break;
+
+		case DRM_FORMAT_RGBA1010102:
+			fourcc = DRM_FORMAT_RGBX1010102;
+			break;
+		}
+	}
+
+	return fourcc;
+}
+
 static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
 {
 	struct rcar_du_vsp_plane_state *state =
@@ -189,7 +220,7 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
 		.alpha = state->state.alpha >> 8,
 		.zpos = state->state.zpos,
 	};
-	u32 fourcc = state->format->fourcc;
+	u32 fourcc = rcar_du_vsp_state_get_format(state);
 	unsigned int i;
 
 	cfg.src.left = state->state.src.x1 >> 16;
@@ -206,22 +237,6 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
 		cfg.mem[i] = sg_dma_address(state->sg_tables[i].sgl)
 			   + fb->offsets[i];
 
-	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
-		switch (fourcc) {
-		case DRM_FORMAT_ARGB1555:
-			fourcc = DRM_FORMAT_XRGB1555;
-			break;
-
-		case DRM_FORMAT_ARGB4444:
-			fourcc = DRM_FORMAT_XRGB4444;
-			break;
-
-		case DRM_FORMAT_ARGB8888:
-			fourcc = DRM_FORMAT_XRGB8888;
-			break;
-		}
-	}
-
 	format = rcar_du_format_info(fourcc);
 	cfg.pixelformat = format->v4l2;
 
-- 
2.25.1

[PATCH 2/2] drm:rcar-du: Enable ABGR and XBGR formats

[Damian Hobson-Garcia]

These formats are used by Android so having them available
allows the DU to be used for composition operations.

Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
---
 drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
index 96241c03b60f..d61bb2e44c06 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
@@ -122,6 +122,8 @@ static const u32 rcar_du_vsp_formats[] = {
 	DRM_FORMAT_RGB888,
 	DRM_FORMAT_BGRA8888,
 	DRM_FORMAT_BGRX8888,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_XBGR8888,
 	DRM_FORMAT_ARGB8888,
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_UYVY,
@@ -154,6 +156,8 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
 	DRM_FORMAT_RGB888,
 	DRM_FORMAT_BGRA8888,
 	DRM_FORMAT_BGRX8888,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_XBGR8888,
 	DRM_FORMAT_ARGB8888,
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_RGBX1010102,
@@ -194,6 +198,10 @@ static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
 			fourcc = DRM_FORMAT_XRGB8888;
 			break;
 
+		case DRM_FORMAT_ABGR8888:
+			fourcc = DRM_FORMAT_XBGR8888;
+			break;
+
 		case DRM_FORMAT_BGRA8888:
 			fourcc = DRM_FORMAT_BGRX8888;
 			break;
-- 
2.25.1

[PATCH 2/2] drm:rcar-du: Enable ABGR and XBGR formats

[Damian Hobson-Garcia]

These formats are used by Android so having them available
allows the DU to be used for composition operations.

Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
---
 drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
index 96241c03b60f..d61bb2e44c06 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
@@ -122,6 +122,8 @@ static const u32 rcar_du_vsp_formats[] = {
 	DRM_FORMAT_RGB888,
 	DRM_FORMAT_BGRA8888,
 	DRM_FORMAT_BGRX8888,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_XBGR8888,
 	DRM_FORMAT_ARGB8888,
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_UYVY,
@@ -154,6 +156,8 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
 	DRM_FORMAT_RGB888,
 	DRM_FORMAT_BGRA8888,
 	DRM_FORMAT_BGRX8888,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_XBGR8888,
 	DRM_FORMAT_ARGB8888,
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_RGBX1010102,
@@ -194,6 +198,10 @@ static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
 			fourcc = DRM_FORMAT_XRGB8888;
 			break;
 
+		case DRM_FORMAT_ABGR8888:
+			fourcc = DRM_FORMAT_XBGR8888;
+			break;
+
 		case DRM_FORMAT_BGRA8888:
 			fourcc = DRM_FORMAT_BGRX8888;
 			break;
-- 
2.25.1

Re: [PATCH 2/2] drm:rcar-du: Enable ABGR and XBGR formats

[Laurent Pinchart]

Hi Damian,

Thank you for the patch.

On Fri, Jul 28, 2023 at 04:07:14PM -0400, Damian Hobson-Garcia wrote:
> These formats are used by Android so having them available
> allows the DU to be used for composition operations.
> 
> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>

This looks good to me. I'm not sure why I haven't included them
initially.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> index 96241c03b60f..d61bb2e44c06 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> @@ -122,6 +122,8 @@ static const u32 rcar_du_vsp_formats[] = {
>  	DRM_FORMAT_RGB888,
>  	DRM_FORMAT_BGRA8888,
>  	DRM_FORMAT_BGRX8888,
> +	DRM_FORMAT_ABGR8888,
> +	DRM_FORMAT_XBGR8888,
>  	DRM_FORMAT_ARGB8888,
>  	DRM_FORMAT_XRGB8888,
>  	DRM_FORMAT_UYVY,
> @@ -154,6 +156,8 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
>  	DRM_FORMAT_RGB888,
>  	DRM_FORMAT_BGRA8888,
>  	DRM_FORMAT_BGRX8888,
> +	DRM_FORMAT_ABGR8888,
> +	DRM_FORMAT_XBGR8888,
>  	DRM_FORMAT_ARGB8888,
>  	DRM_FORMAT_XRGB8888,
>  	DRM_FORMAT_RGBX1010102,
> @@ -194,6 +198,10 @@ static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
>  			fourcc = DRM_FORMAT_XRGB8888;
>  			break;
>  
> +		case DRM_FORMAT_ABGR8888:
> +			fourcc = DRM_FORMAT_XBGR8888;
> +			break;
> +
>  		case DRM_FORMAT_BGRA8888:
>  			fourcc = DRM_FORMAT_BGRX8888;
>  			break;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/2] drm:rcar-du: Enable ABGR and XBGR formats

[Laurent Pinchart]

Hi Damian,

Thank you for the patch.

On Fri, Jul 28, 2023 at 04:07:14PM -0400, Damian Hobson-Garcia wrote:
> These formats are used by Android so having them available
> allows the DU to be used for composition operations.
> 
> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>

This looks good to me. I'm not sure why I haven't included them
initially.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> index 96241c03b60f..d61bb2e44c06 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> @@ -122,6 +122,8 @@ static const u32 rcar_du_vsp_formats[] = {
>  	DRM_FORMAT_RGB888,
>  	DRM_FORMAT_BGRA8888,
>  	DRM_FORMAT_BGRX8888,
> +	DRM_FORMAT_ABGR8888,
> +	DRM_FORMAT_XBGR8888,
>  	DRM_FORMAT_ARGB8888,
>  	DRM_FORMAT_XRGB8888,
>  	DRM_FORMAT_UYVY,
> @@ -154,6 +156,8 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
>  	DRM_FORMAT_RGB888,
>  	DRM_FORMAT_BGRA8888,
>  	DRM_FORMAT_BGRX8888,
> +	DRM_FORMAT_ABGR8888,
> +	DRM_FORMAT_XBGR8888,
>  	DRM_FORMAT_ARGB8888,
>  	DRM_FORMAT_XRGB8888,
>  	DRM_FORMAT_RGBX1010102,
> @@ -194,6 +198,10 @@ static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
>  			fourcc = DRM_FORMAT_XRGB8888;
>  			break;
>  
> +		case DRM_FORMAT_ABGR8888:
> +			fourcc = DRM_FORMAT_XBGR8888;
> +			break;
> +
>  		case DRM_FORMAT_BGRA8888:
>  			fourcc = DRM_FORMAT_BGRX8888;
>  			break;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Laurent Pinchart]

Hi Damian,

Thank you for the patch.

On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote:
> Add additional pixel formats for which blending is disabling when

Did you mean "disabled" instead of "disabling" ?

> DRM_MODE_BLEND_PIXEL_NONE is set.
> 
> Refactor the fourcc selection into a separate function to handle the
> increased number of formats.
> 
> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> ---
>  drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
>  1 file changed, 32 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> index 45c05d0ffc70..96241c03b60f 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
>  	DRM_FORMAT_Y212,
>  };
>  
> +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
> +{
> +	u32 fourcc = state->format->fourcc;
> +
> +	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> +		switch (fourcc) {
> +		case DRM_FORMAT_ARGB1555:
> +			fourcc = DRM_FORMAT_XRGB1555;
> +			break;
> +
> +		case DRM_FORMAT_ARGB4444:
> +			fourcc = DRM_FORMAT_XRGB4444;
> +			break;
> +
> +		case DRM_FORMAT_ARGB8888:
> +			fourcc = DRM_FORMAT_XRGB8888;
> +			break;
> +
> +		case DRM_FORMAT_BGRA8888:
> +			fourcc = DRM_FORMAT_BGRX8888;
> +			break;
> +
> +		case DRM_FORMAT_RGBA1010102:
> +			fourcc = DRM_FORMAT_RGBX1010102;
> +			break;

Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out
intentionally ?

> +		}
> +	}
> +
> +	return fourcc;
> +}
> +
>  static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
>  {
>  	struct rcar_du_vsp_plane_state *state =
> @@ -189,7 +220,7 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
>  		.alpha = state->state.alpha >> 8,
>  		.zpos = state->state.zpos,
>  	};
> -	u32 fourcc = state->format->fourcc;
> +	u32 fourcc = rcar_du_vsp_state_get_format(state);
>  	unsigned int i;
>  
>  	cfg.src.left = state->state.src.x1 >> 16;
> @@ -206,22 +237,6 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
>  		cfg.mem[i] = sg_dma_address(state->sg_tables[i].sgl)
>  			   + fb->offsets[i];
>  
> -	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> -		switch (fourcc) {
> -		case DRM_FORMAT_ARGB1555:
> -			fourcc = DRM_FORMAT_XRGB1555;
> -			break;
> -
> -		case DRM_FORMAT_ARGB4444:
> -			fourcc = DRM_FORMAT_XRGB4444;
> -			break;
> -
> -		case DRM_FORMAT_ARGB8888:
> -			fourcc = DRM_FORMAT_XRGB8888;
> -			break;
> -		}
> -	}
> -
>  	format = rcar_du_format_info(fourcc);
>  	cfg.pixelformat = format->v4l2;
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Laurent Pinchart]

Hi Damian,

Thank you for the patch.

On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote:
> Add additional pixel formats for which blending is disabling when

Did you mean "disabled" instead of "disabling" ?

> DRM_MODE_BLEND_PIXEL_NONE is set.
> 
> Refactor the fourcc selection into a separate function to handle the
> increased number of formats.
> 
> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> ---
>  drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
>  1 file changed, 32 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> index 45c05d0ffc70..96241c03b60f 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
>  	DRM_FORMAT_Y212,
>  };
>  
> +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
> +{
> +	u32 fourcc = state->format->fourcc;
> +
> +	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> +		switch (fourcc) {
> +		case DRM_FORMAT_ARGB1555:
> +			fourcc = DRM_FORMAT_XRGB1555;
> +			break;
> +
> +		case DRM_FORMAT_ARGB4444:
> +			fourcc = DRM_FORMAT_XRGB4444;
> +			break;
> +
> +		case DRM_FORMAT_ARGB8888:
> +			fourcc = DRM_FORMAT_XRGB8888;
> +			break;
> +
> +		case DRM_FORMAT_BGRA8888:
> +			fourcc = DRM_FORMAT_BGRX8888;
> +			break;
> +
> +		case DRM_FORMAT_RGBA1010102:
> +			fourcc = DRM_FORMAT_RGBX1010102;
> +			break;

Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out
intentionally ?

> +		}
> +	}
> +
> +	return fourcc;
> +}
> +
>  static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
>  {
>  	struct rcar_du_vsp_plane_state *state =
> @@ -189,7 +220,7 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
>  		.alpha = state->state.alpha >> 8,
>  		.zpos = state->state.zpos,
>  	};
> -	u32 fourcc = state->format->fourcc;
> +	u32 fourcc = rcar_du_vsp_state_get_format(state);
>  	unsigned int i;
>  
>  	cfg.src.left = state->state.src.x1 >> 16;
> @@ -206,22 +237,6 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
>  		cfg.mem[i] = sg_dma_address(state->sg_tables[i].sgl)
>  			   + fb->offsets[i];
>  
> -	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> -		switch (fourcc) {
> -		case DRM_FORMAT_ARGB1555:
> -			fourcc = DRM_FORMAT_XRGB1555;
> -			break;
> -
> -		case DRM_FORMAT_ARGB4444:
> -			fourcc = DRM_FORMAT_XRGB4444;
> -			break;
> -
> -		case DRM_FORMAT_ARGB8888:
> -			fourcc = DRM_FORMAT_XRGB8888;
> -			break;
> -		}
> -	}
> -
>  	format = rcar_du_format_info(fourcc);
>  	cfg.pixelformat = format->v4l2;
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Laurent Pinchart]

On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote:
> Hi Damian,
> 
> Thank you for the patch.
> 
> On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote:
> > Add additional pixel formats for which blending is disabling when
> 
> Did you mean "disabled" instead of "disabling" ?
> 
> > DRM_MODE_BLEND_PIXEL_NONE is set.
> > 
> > Refactor the fourcc selection into a separate function to handle the
> > increased number of formats.
> > 
> > Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> > ---
> >  drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
> >  1 file changed, 32 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > index 45c05d0ffc70..96241c03b60f 100644
> > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
> >  	DRM_FORMAT_Y212,
> >  };
> >  
> > +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
> > +{
> > +	u32 fourcc = state->format->fourcc;
> > +
> > +	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> > +		switch (fourcc) {
> > +		case DRM_FORMAT_ARGB1555:
> > +			fourcc = DRM_FORMAT_XRGB1555;
> > +			break;
> > +
> > +		case DRM_FORMAT_ARGB4444:
> > +			fourcc = DRM_FORMAT_XRGB4444;
> > +			break;
> > +
> > +		case DRM_FORMAT_ARGB8888:
> > +			fourcc = DRM_FORMAT_XRGB8888;
> > +			break;
> > +
> > +		case DRM_FORMAT_BGRA8888:
> > +			fourcc = DRM_FORMAT_BGRX8888;
> > +			break;
> > +
> > +		case DRM_FORMAT_RGBA1010102:
> > +			fourcc = DRM_FORMAT_RGBX1010102;
> > +			break;
> 
> Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out
> intentionally ?

It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as
DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment.
Let's do so with a patch on top of this series. There's no need to send
a v2, I can handle the simple change in the commit message if you let me
know whether my comment is right or wrong.

> > +		}
> > +	}
> > +
> > +	return fourcc;
> > +}
> > +
> >  static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
> >  {
> >  	struct rcar_du_vsp_plane_state *state =
> > @@ -189,7 +220,7 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
> >  		.alpha = state->state.alpha >> 8,
> >  		.zpos = state->state.zpos,
> >  	};
> > -	u32 fourcc = state->format->fourcc;
> > +	u32 fourcc = rcar_du_vsp_state_get_format(state);
> >  	unsigned int i;
> >  
> >  	cfg.src.left = state->state.src.x1 >> 16;
> > @@ -206,22 +237,6 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
> >  		cfg.mem[i] = sg_dma_address(state->sg_tables[i].sgl)
> >  			   + fb->offsets[i];
> >  
> > -	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> > -		switch (fourcc) {
> > -		case DRM_FORMAT_ARGB1555:
> > -			fourcc = DRM_FORMAT_XRGB1555;
> > -			break;
> > -
> > -		case DRM_FORMAT_ARGB4444:
> > -			fourcc = DRM_FORMAT_XRGB4444;
> > -			break;
> > -
> > -		case DRM_FORMAT_ARGB8888:
> > -			fourcc = DRM_FORMAT_XRGB8888;
> > -			break;
> > -		}
> > -	}
> > -
> >  	format = rcar_du_format_info(fourcc);
> >  	cfg.pixelformat = format->v4l2;
> >  

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Laurent Pinchart]

On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote:
> Hi Damian,
> 
> Thank you for the patch.
> 
> On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote:
> > Add additional pixel formats for which blending is disabling when
> 
> Did you mean "disabled" instead of "disabling" ?
> 
> > DRM_MODE_BLEND_PIXEL_NONE is set.
> > 
> > Refactor the fourcc selection into a separate function to handle the
> > increased number of formats.
> > 
> > Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> > ---
> >  drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
> >  1 file changed, 32 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > index 45c05d0ffc70..96241c03b60f 100644
> > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
> >  	DRM_FORMAT_Y212,
> >  };
> >  
> > +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
> > +{
> > +	u32 fourcc = state->format->fourcc;
> > +
> > +	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> > +		switch (fourcc) {
> > +		case DRM_FORMAT_ARGB1555:
> > +			fourcc = DRM_FORMAT_XRGB1555;
> > +			break;
> > +
> > +		case DRM_FORMAT_ARGB4444:
> > +			fourcc = DRM_FORMAT_XRGB4444;
> > +			break;
> > +
> > +		case DRM_FORMAT_ARGB8888:
> > +			fourcc = DRM_FORMAT_XRGB8888;
> > +			break;
> > +
> > +		case DRM_FORMAT_BGRA8888:
> > +			fourcc = DRM_FORMAT_BGRX8888;
> > +			break;
> > +
> > +		case DRM_FORMAT_RGBA1010102:
> > +			fourcc = DRM_FORMAT_RGBX1010102;
> > +			break;
> 
> Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out
> intentionally ?

It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as
DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment.
Let's do so with a patch on top of this series. There's no need to send
a v2, I can handle the simple change in the commit message if you let me
know whether my comment is right or wrong.

> > +		}
> > +	}
> > +
> > +	return fourcc;
> > +}
> > +
> >  static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
> >  {
> >  	struct rcar_du_vsp_plane_state *state =
> > @@ -189,7 +220,7 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
> >  		.alpha = state->state.alpha >> 8,
> >  		.zpos = state->state.zpos,
> >  	};
> > -	u32 fourcc = state->format->fourcc;
> > +	u32 fourcc = rcar_du_vsp_state_get_format(state);
> >  	unsigned int i;
> >  
> >  	cfg.src.left = state->state.src.x1 >> 16;
> > @@ -206,22 +237,6 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
> >  		cfg.mem[i] = sg_dma_address(state->sg_tables[i].sgl)
> >  			   + fb->offsets[i];
> >  
> > -	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> > -		switch (fourcc) {
> > -		case DRM_FORMAT_ARGB1555:
> > -			fourcc = DRM_FORMAT_XRGB1555;
> > -			break;
> > -
> > -		case DRM_FORMAT_ARGB4444:
> > -			fourcc = DRM_FORMAT_XRGB4444;
> > -			break;
> > -
> > -		case DRM_FORMAT_ARGB8888:
> > -			fourcc = DRM_FORMAT_XRGB8888;
> > -			break;
> > -		}
> > -	}
> > -
> >  	format = rcar_du_format_info(fourcc);
> >  	cfg.pixelformat = format->v4l2;
> >  

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Laurent Pinchart]

On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote:
> On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote:
> > Hi Damian,
> > 
> > Thank you for the patch.
> > 
> > On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote:
> > > Add additional pixel formats for which blending is disabling when
> > 
> > Did you mean "disabled" instead of "disabling" ?
> > 
> > > DRM_MODE_BLEND_PIXEL_NONE is set.
> > > 
> > > Refactor the fourcc selection into a separate function to handle the
> > > increased number of formats.
> > > 
> > > Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> > > ---
> > >  drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
> > >  1 file changed, 32 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > > index 45c05d0ffc70..96241c03b60f 100644
> > > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > > @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
> > >  	DRM_FORMAT_Y212,
> > >  };
> > >  
> > > +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
> > > +{
> > > +	u32 fourcc = state->format->fourcc;
> > > +
> > > +	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> > > +		switch (fourcc) {
> > > +		case DRM_FORMAT_ARGB1555:
> > > +			fourcc = DRM_FORMAT_XRGB1555;
> > > +			break;
> > > +
> > > +		case DRM_FORMAT_ARGB4444:
> > > +			fourcc = DRM_FORMAT_XRGB4444;
> > > +			break;
> > > +
> > > +		case DRM_FORMAT_ARGB8888:
> > > +			fourcc = DRM_FORMAT_XRGB8888;
> > > +			break;
> > > +
> > > +		case DRM_FORMAT_BGRA8888:
> > > +			fourcc = DRM_FORMAT_BGRX8888;
> > > +			break;
> > > +
> > > +		case DRM_FORMAT_RGBA1010102:
> > > +			fourcc = DRM_FORMAT_RGBX1010102;
> > > +			break;
> > 
> > Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out
> > intentionally ?
> 
> It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as
> DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment.
> Let's do so with a patch on top of this series.

Replying to myself again, the datasheet doesn't explicitly list
DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to
specify the location of the components should work fine for that format.
Is this something you would be able to test ?

> There's no need to send
> a v2, I can handle the simple change in the commit message if you let me
> know whether my comment is right or wrong.
> 
> > > +		}
> > > +	}
> > > +
> > > +	return fourcc;
> > > +}
> > > +
> > >  static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
> > >  {
> > >  	struct rcar_du_vsp_plane_state *state =
> > > @@ -189,7 +220,7 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
> > >  		.alpha = state->state.alpha >> 8,
> > >  		.zpos = state->state.zpos,
> > >  	};
> > > -	u32 fourcc = state->format->fourcc;
> > > +	u32 fourcc = rcar_du_vsp_state_get_format(state);
> > >  	unsigned int i;
> > >  
> > >  	cfg.src.left = state->state.src.x1 >> 16;
> > > @@ -206,22 +237,6 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
> > >  		cfg.mem[i] = sg_dma_address(state->sg_tables[i].sgl)
> > >  			   + fb->offsets[i];
> > >  
> > > -	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> > > -		switch (fourcc) {
> > > -		case DRM_FORMAT_ARGB1555:
> > > -			fourcc = DRM_FORMAT_XRGB1555;
> > > -			break;
> > > -
> > > -		case DRM_FORMAT_ARGB4444:
> > > -			fourcc = DRM_FORMAT_XRGB4444;
> > > -			break;
> > > -
> > > -		case DRM_FORMAT_ARGB8888:
> > > -			fourcc = DRM_FORMAT_XRGB8888;
> > > -			break;
> > > -		}
> > > -	}
> > > -
> > >  	format = rcar_du_format_info(fourcc);
> > >  	cfg.pixelformat = format->v4l2;
> > >  

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Laurent Pinchart]

On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote:
> On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote:
> > Hi Damian,
> > 
> > Thank you for the patch.
> > 
> > On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote:
> > > Add additional pixel formats for which blending is disabling when
> > 
> > Did you mean "disabled" instead of "disabling" ?
> > 
> > > DRM_MODE_BLEND_PIXEL_NONE is set.
> > > 
> > > Refactor the fourcc selection into a separate function to handle the
> > > increased number of formats.
> > > 
> > > Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> > > ---
> > >  drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
> > >  1 file changed, 32 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > > index 45c05d0ffc70..96241c03b60f 100644
> > > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > > @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
> > >  	DRM_FORMAT_Y212,
> > >  };
> > >  
> > > +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
> > > +{
> > > +	u32 fourcc = state->format->fourcc;
> > > +
> > > +	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> > > +		switch (fourcc) {
> > > +		case DRM_FORMAT_ARGB1555:
> > > +			fourcc = DRM_FORMAT_XRGB1555;
> > > +			break;
> > > +
> > > +		case DRM_FORMAT_ARGB4444:
> > > +			fourcc = DRM_FORMAT_XRGB4444;
> > > +			break;
> > > +
> > > +		case DRM_FORMAT_ARGB8888:
> > > +			fourcc = DRM_FORMAT_XRGB8888;
> > > +			break;
> > > +
> > > +		case DRM_FORMAT_BGRA8888:
> > > +			fourcc = DRM_FORMAT_BGRX8888;
> > > +			break;
> > > +
> > > +		case DRM_FORMAT_RGBA1010102:
> > > +			fourcc = DRM_FORMAT_RGBX1010102;
> > > +			break;
> > 
> > Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out
> > intentionally ?
> 
> It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as
> DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment.
> Let's do so with a patch on top of this series.

Replying to myself again, the datasheet doesn't explicitly list
DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to
specify the location of the components should work fine for that format.
Is this something you would be able to test ?

> There's no need to send
> a v2, I can handle the simple change in the commit message if you let me
> know whether my comment is right or wrong.
> 
> > > +		}
> > > +	}
> > > +
> > > +	return fourcc;
> > > +}
> > > +
> > >  static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
> > >  {
> > >  	struct rcar_du_vsp_plane_state *state =
> > > @@ -189,7 +220,7 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
> > >  		.alpha = state->state.alpha >> 8,
> > >  		.zpos = state->state.zpos,
> > >  	};
> > > -	u32 fourcc = state->format->fourcc;
> > > +	u32 fourcc = rcar_du_vsp_state_get_format(state);
> > >  	unsigned int i;
> > >  
> > >  	cfg.src.left = state->state.src.x1 >> 16;
> > > @@ -206,22 +237,6 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
> > >  		cfg.mem[i] = sg_dma_address(state->sg_tables[i].sgl)
> > >  			   + fb->offsets[i];
> > >  
> > > -	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> > > -		switch (fourcc) {
> > > -		case DRM_FORMAT_ARGB1555:
> > > -			fourcc = DRM_FORMAT_XRGB1555;
> > > -			break;
> > > -
> > > -		case DRM_FORMAT_ARGB4444:
> > > -			fourcc = DRM_FORMAT_XRGB4444;
> > > -			break;
> > > -
> > > -		case DRM_FORMAT_ARGB8888:
> > > -			fourcc = DRM_FORMAT_XRGB8888;
> > > -			break;
> > > -		}
> > > -	}
> > > -
> > >  	format = rcar_du_format_info(fourcc);
> > >  	cfg.pixelformat = format->v4l2;
> > >  

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Damian Hobson-Garcia]

Hi Laurent,

Thank you for the review.

On 2023/08/03 20:06, Laurent Pinchart wrote:
> On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote:
>> On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote:
>>> Hi Damian,
>>>
>>> Thank you for the patch.
>>>
>>> On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote:
>>>> Add additional pixel formats for which blending is disabling when
>>> Did you mean "disabled" instead of "disabling" ?


Oops.  Yes, that's exactly what I meant.


>>>
>>>> DRM_MODE_BLEND_PIXEL_NONE is set.
>>>>
>>>> Refactor the fourcc selection into a separate function to handle the
>>>> increased number of formats.
>>>>
>>>> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
>>>> ---
>>>>   drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
>>>>   1 file changed, 32 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
>>>> index 45c05d0ffc70..96241c03b60f 100644
>>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
>>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
>>>> @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
>>>>   	DRM_FORMAT_Y212,
>>>>   };
>>>>   
>>>> +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
>>>> +{
>>>> +	u32 fourcc = state->format->fourcc;
>>>> +
>>>> +	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
>>>> +		switch (fourcc) {
>>>> +		case DRM_FORMAT_ARGB1555:
>>>> +			fourcc = DRM_FORMAT_XRGB1555;
>>>> +			break;
>>>> +
>>>> +		case DRM_FORMAT_ARGB4444:
>>>> +			fourcc = DRM_FORMAT_XRGB4444;
>>>> +			break;
>>>> +
>>>> +		case DRM_FORMAT_ARGB8888:
>>>> +			fourcc = DRM_FORMAT_XRGB8888;
>>>> +			break;
>>>> +
>>>> +		case DRM_FORMAT_BGRA8888:
>>>> +			fourcc = DRM_FORMAT_BGRX8888;
>>>> +			break;
>>>> +
>>>> +		case DRM_FORMAT_RGBA1010102:
>>>> +			fourcc = DRM_FORMAT_RGBX1010102;
>>>> +			break;
>>> Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out
>>> intentionally ?

Yes, it was intentionally left out for the time being for the
reason that you noted (i.e. DRM_FORMAT_XRGB2101010 not
being handled by the DU driver).

>> It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as
>> DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment.
>> Let's do so with a patch on top of this series.
Yes, I was thinking the same thing.
> Replying to myself again, the datasheet doesn't explicitly list
> DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to
> specify the location of the components should work fine for that format.
> Is this something you would be able to test ?

Unfortunately I don't have a Gen 4 system on hand to test the 2-10-10-10 
formats with.
I will double-check with the office in Japan, but I don't think that 
they have one either.

>
>> There's no need to send
>> a v2, I can handle the simple change in the commit message if you let me
>> know whether my comment is right or wrong.


Thank you,
Damian

Re: [PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Damian Hobson-Garcia]

Hi Laurent,

Thank you for the review.

On 2023/08/03 20:06, Laurent Pinchart wrote:
> On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote:
>> On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote:
>>> Hi Damian,
>>>
>>> Thank you for the patch.
>>>
>>> On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote:
>>>> Add additional pixel formats for which blending is disabling when
>>> Did you mean "disabled" instead of "disabling" ?


Oops.  Yes, that's exactly what I meant.


>>>
>>>> DRM_MODE_BLEND_PIXEL_NONE is set.
>>>>
>>>> Refactor the fourcc selection into a separate function to handle the
>>>> increased number of formats.
>>>>
>>>> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
>>>> ---
>>>>   drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
>>>>   1 file changed, 32 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
>>>> index 45c05d0ffc70..96241c03b60f 100644
>>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
>>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
>>>> @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
>>>>   	DRM_FORMAT_Y212,
>>>>   };
>>>>   
>>>> +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
>>>> +{
>>>> +	u32 fourcc = state->format->fourcc;
>>>> +
>>>> +	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
>>>> +		switch (fourcc) {
>>>> +		case DRM_FORMAT_ARGB1555:
>>>> +			fourcc = DRM_FORMAT_XRGB1555;
>>>> +			break;
>>>> +
>>>> +		case DRM_FORMAT_ARGB4444:
>>>> +			fourcc = DRM_FORMAT_XRGB4444;
>>>> +			break;
>>>> +
>>>> +		case DRM_FORMAT_ARGB8888:
>>>> +			fourcc = DRM_FORMAT_XRGB8888;
>>>> +			break;
>>>> +
>>>> +		case DRM_FORMAT_BGRA8888:
>>>> +			fourcc = DRM_FORMAT_BGRX8888;
>>>> +			break;
>>>> +
>>>> +		case DRM_FORMAT_RGBA1010102:
>>>> +			fourcc = DRM_FORMAT_RGBX1010102;
>>>> +			break;
>>> Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out
>>> intentionally ?

Yes, it was intentionally left out for the time being for the
reason that you noted (i.e. DRM_FORMAT_XRGB2101010 not
being handled by the DU driver).

>> It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as
>> DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment.
>> Let's do so with a patch on top of this series.
Yes, I was thinking the same thing.
> Replying to myself again, the datasheet doesn't explicitly list
> DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to
> specify the location of the components should work fine for that format.
> Is this something you would be able to test ?

Unfortunately I don't have a Gen 4 system on hand to test the 2-10-10-10 
formats with.
I will double-check with the office in Japan, but I don't think that 
they have one either.

>
>> There's no need to send
>> a v2, I can handle the simple change in the commit message if you let me
>> know whether my comment is right or wrong.


Thank you,
Damian

Re: [PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Laurent Pinchart]

Hi Damian,

(CC'ing Tomi)

On Fri, Aug 04, 2023 at 11:49:32AM -0400, Damian Hobson-Garcia wrote:
> On 2023/08/03 20:06, Laurent Pinchart wrote:
> > On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote:
> >> On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote:
> >>> Hi Damian,
> >>>
> >>> Thank you for the patch.
> >>>
> >>> On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote:
> >>>> Add additional pixel formats for which blending is disabling when
> >>> Did you mean "disabled" instead of "disabling" ?
> 
> Oops.  Yes, that's exactly what I meant.
> 
> >>>
> >>>> DRM_MODE_BLEND_PIXEL_NONE is set.
> >>>>
> >>>> Refactor the fourcc selection into a separate function to handle the
> >>>> increased number of formats.
> >>>>
> >>>> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> >>>> ---
> >>>>   drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
> >>>>   1 file changed, 32 insertions(+), 17 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> >>>> index 45c05d0ffc70..96241c03b60f 100644
> >>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> >>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> >>>> @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
> >>>>   	DRM_FORMAT_Y212,
> >>>>   };
> >>>>   
> >>>> +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
> >>>> +{
> >>>> +	u32 fourcc = state->format->fourcc;
> >>>> +
> >>>> +	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> >>>> +		switch (fourcc) {
> >>>> +		case DRM_FORMAT_ARGB1555:
> >>>> +			fourcc = DRM_FORMAT_XRGB1555;
> >>>> +			break;
> >>>> +
> >>>> +		case DRM_FORMAT_ARGB4444:
> >>>> +			fourcc = DRM_FORMAT_XRGB4444;
> >>>> +			break;
> >>>> +
> >>>> +		case DRM_FORMAT_ARGB8888:
> >>>> +			fourcc = DRM_FORMAT_XRGB8888;
> >>>> +			break;
> >>>> +
> >>>> +		case DRM_FORMAT_BGRA8888:
> >>>> +			fourcc = DRM_FORMAT_BGRX8888;
> >>>> +			break;
> >>>> +
> >>>> +		case DRM_FORMAT_RGBA1010102:
> >>>> +			fourcc = DRM_FORMAT_RGBX1010102;
> >>>> +			break;
> >>> Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out
> >>> intentionally ?
> 
> Yes, it was intentionally left out for the time being for the
> reason that you noted (i.e. DRM_FORMAT_XRGB2101010 not
> being handled by the DU driver).
> 
> >> It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as
> >> DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment.
> >> Let's do so with a patch on top of this series.
> Yes, I was thinking the same thing.
> > Replying to myself again, the datasheet doesn't explicitly list
> > DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to
> > specify the location of the components should work fine for that format.
> > Is this something you would be able to test ?
> 
> Unfortunately I don't have a Gen 4 system on hand to test the 2-10-10-10 
> formats with.
> I will double-check with the office in Japan, but I don't think that 
> they have one either.

Tomi, is this something you could test ?

> >> There's no need to send
> >> a v2, I can handle the simple change in the commit message if you let me
> >> know whether my comment is right or wrong.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Laurent Pinchart]

Hi Damian,

(CC'ing Tomi)

On Fri, Aug 04, 2023 at 11:49:32AM -0400, Damian Hobson-Garcia wrote:
> On 2023/08/03 20:06, Laurent Pinchart wrote:
> > On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote:
> >> On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote:
> >>> Hi Damian,
> >>>
> >>> Thank you for the patch.
> >>>
> >>> On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote:
> >>>> Add additional pixel formats for which blending is disabling when
> >>> Did you mean "disabled" instead of "disabling" ?
> 
> Oops.  Yes, that's exactly what I meant.
> 
> >>>
> >>>> DRM_MODE_BLEND_PIXEL_NONE is set.
> >>>>
> >>>> Refactor the fourcc selection into a separate function to handle the
> >>>> increased number of formats.
> >>>>
> >>>> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> >>>> ---
> >>>>   drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
> >>>>   1 file changed, 32 insertions(+), 17 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> >>>> index 45c05d0ffc70..96241c03b60f 100644
> >>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> >>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> >>>> @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
> >>>>   	DRM_FORMAT_Y212,
> >>>>   };
> >>>>   
> >>>> +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
> >>>> +{
> >>>> +	u32 fourcc = state->format->fourcc;
> >>>> +
> >>>> +	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> >>>> +		switch (fourcc) {
> >>>> +		case DRM_FORMAT_ARGB1555:
> >>>> +			fourcc = DRM_FORMAT_XRGB1555;
> >>>> +			break;
> >>>> +
> >>>> +		case DRM_FORMAT_ARGB4444:
> >>>> +			fourcc = DRM_FORMAT_XRGB4444;
> >>>> +			break;
> >>>> +
> >>>> +		case DRM_FORMAT_ARGB8888:
> >>>> +			fourcc = DRM_FORMAT_XRGB8888;
> >>>> +			break;
> >>>> +
> >>>> +		case DRM_FORMAT_BGRA8888:
> >>>> +			fourcc = DRM_FORMAT_BGRX8888;
> >>>> +			break;
> >>>> +
> >>>> +		case DRM_FORMAT_RGBA1010102:
> >>>> +			fourcc = DRM_FORMAT_RGBX1010102;
> >>>> +			break;
> >>> Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out
> >>> intentionally ?
> 
> Yes, it was intentionally left out for the time being for the
> reason that you noted (i.e. DRM_FORMAT_XRGB2101010 not
> being handled by the DU driver).
> 
> >> It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as
> >> DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment.
> >> Let's do so with a patch on top of this series.
> Yes, I was thinking the same thing.
> > Replying to myself again, the datasheet doesn't explicitly list
> > DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to
> > specify the location of the components should work fine for that format.
> > Is this something you would be able to test ?
> 
> Unfortunately I don't have a Gen 4 system on hand to test the 2-10-10-10 
> formats with.
> I will double-check with the office in Japan, but I don't think that 
> they have one either.

Tomi, is this something you could test ?

> >> There's no need to send
> >> a v2, I can handle the simple change in the commit message if you let me
> >> know whether my comment is right or wrong.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Laurent Pinchart]

On Fri, Aug 04, 2023 at 06:52:51PM +0300, Laurent Pinchart wrote:
> On Fri, Aug 04, 2023 at 11:49:32AM -0400, Damian Hobson-Garcia wrote:
> > On 2023/08/03 20:06, Laurent Pinchart wrote:
> > > On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote:
> > >> On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote:
> > >>> On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote:
> > >>>> Add additional pixel formats for which blending is disabling when
> > >>>
> > >>> Did you mean "disabled" instead of "disabling" ?
> > 
> > Oops.  Yes, that's exactly what I meant.

I'll fix this locally.

> > >>>> DRM_MODE_BLEND_PIXEL_NONE is set.
> > >>>>
> > >>>> Refactor the fourcc selection into a separate function to handle the
> > >>>> increased number of formats.
> > >>>>
> > >>>> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> > >>>> ---
> > >>>>   drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
> > >>>>   1 file changed, 32 insertions(+), 17 deletions(-)
> > >>>>
> > >>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > >>>> index 45c05d0ffc70..96241c03b60f 100644
> > >>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > >>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > >>>> @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
> > >>>>   	DRM_FORMAT_Y212,
> > >>>>   };
> > >>>>   
> > >>>> +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
> > >>>> +{
> > >>>> +	u32 fourcc = state->format->fourcc;
> > >>>> +
> > >>>> +	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> > >>>> +		switch (fourcc) {
> > >>>> +		case DRM_FORMAT_ARGB1555:
> > >>>> +			fourcc = DRM_FORMAT_XRGB1555;
> > >>>> +			break;
> > >>>> +
> > >>>> +		case DRM_FORMAT_ARGB4444:
> > >>>> +			fourcc = DRM_FORMAT_XRGB4444;
> > >>>> +			break;
> > >>>> +
> > >>>> +		case DRM_FORMAT_ARGB8888:
> > >>>> +			fourcc = DRM_FORMAT_XRGB8888;
> > >>>> +			break;
> > >>>> +
> > >>>> +		case DRM_FORMAT_BGRA8888:
> > >>>> +			fourcc = DRM_FORMAT_BGRX8888;
> > >>>> +			break;
> > >>>> +
> > >>>> +		case DRM_FORMAT_RGBA1010102:
> > >>>> +			fourcc = DRM_FORMAT_RGBX1010102;
> > >>>> +			break;
> > >>>
> > >>> Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out
> > >>> intentionally ?
> > 
> > Yes, it was intentionally left out for the time being for the
> > reason that you noted (i.e. DRM_FORMAT_XRGB2101010 not
> > being handled by the DU driver).
> > 
> > >> It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as
> > >> DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment.
> > >> Let's do so with a patch on top of this series.
> >
> > Yes, I was thinking the same thing.
> >
> > > Replying to myself again, the datasheet doesn't explicitly list
> > > DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to
> > > specify the location of the components should work fine for that format.
> > > Is this something you would be able to test ?
> > 
> > Unfortunately I don't have a Gen 4 system on hand to test the 2-10-10-10 
> > formats with.
> > I will double-check with the office in Japan, but I don't think that 
> > they have one either.
> 
> Tomi, is this something you could test ?

Replying to myself, this is something we could test, but let's not delay
this series, new formats can always be added on top.

> > >> There's no need to send
> > >> a v2, I can handle the simple change in the commit message if you let me
> > >> know whether my comment is right or wrong.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Laurent Pinchart]

On Fri, Aug 04, 2023 at 06:52:51PM +0300, Laurent Pinchart wrote:
> On Fri, Aug 04, 2023 at 11:49:32AM -0400, Damian Hobson-Garcia wrote:
> > On 2023/08/03 20:06, Laurent Pinchart wrote:
> > > On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote:
> > >> On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote:
> > >>> On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote:
> > >>>> Add additional pixel formats for which blending is disabling when
> > >>>
> > >>> Did you mean "disabled" instead of "disabling" ?
> > 
> > Oops.  Yes, that's exactly what I meant.

I'll fix this locally.

> > >>>> DRM_MODE_BLEND_PIXEL_NONE is set.
> > >>>>
> > >>>> Refactor the fourcc selection into a separate function to handle the
> > >>>> increased number of formats.
> > >>>>
> > >>>> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> > >>>> ---
> > >>>>   drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
> > >>>>   1 file changed, 32 insertions(+), 17 deletions(-)
> > >>>>
> > >>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > >>>> index 45c05d0ffc70..96241c03b60f 100644
> > >>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > >>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > >>>> @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
> > >>>>   	DRM_FORMAT_Y212,
> > >>>>   };
> > >>>>   
> > >>>> +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
> > >>>> +{
> > >>>> +	u32 fourcc = state->format->fourcc;
> > >>>> +
> > >>>> +	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> > >>>> +		switch (fourcc) {
> > >>>> +		case DRM_FORMAT_ARGB1555:
> > >>>> +			fourcc = DRM_FORMAT_XRGB1555;
> > >>>> +			break;
> > >>>> +
> > >>>> +		case DRM_FORMAT_ARGB4444:
> > >>>> +			fourcc = DRM_FORMAT_XRGB4444;
> > >>>> +			break;
> > >>>> +
> > >>>> +		case DRM_FORMAT_ARGB8888:
> > >>>> +			fourcc = DRM_FORMAT_XRGB8888;
> > >>>> +			break;
> > >>>> +
> > >>>> +		case DRM_FORMAT_BGRA8888:
> > >>>> +			fourcc = DRM_FORMAT_BGRX8888;
> > >>>> +			break;
> > >>>> +
> > >>>> +		case DRM_FORMAT_RGBA1010102:
> > >>>> +			fourcc = DRM_FORMAT_RGBX1010102;
> > >>>> +			break;
> > >>>
> > >>> Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out
> > >>> intentionally ?
> > 
> > Yes, it was intentionally left out for the time being for the
> > reason that you noted (i.e. DRM_FORMAT_XRGB2101010 not
> > being handled by the DU driver).
> > 
> > >> It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as
> > >> DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment.
> > >> Let's do so with a patch on top of this series.
> >
> > Yes, I was thinking the same thing.
> >
> > > Replying to myself again, the datasheet doesn't explicitly list
> > > DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to
> > > specify the location of the components should work fine for that format.
> > > Is this something you would be able to test ?
> > 
> > Unfortunately I don't have a Gen 4 system on hand to test the 2-10-10-10 
> > formats with.
> > I will double-check with the office in Japan, but I don't think that 
> > they have one either.
> 
> Tomi, is this something you could test ?

Replying to myself, this is something we could test, but let's not delay
this series, new formats can always be added on top.

> > >> There's no need to send
> > >> a v2, I can handle the simple change in the commit message if you let me
> > >> know whether my comment is right or wrong.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Damian Hobson-Garcia]

Hi Laurent,


On 2023/08/14 6:46, Laurent Pinchart wrote:
> On Fri, Aug 04, 2023 at 06:52:51PM +0300, Laurent Pinchart wrote:
>> On Fri, Aug 04, 2023 at 11:49:32AM -0400, Damian Hobson-Garcia wrote:
>>> On 2023/08/03 20:06, Laurent Pinchart wrote:
>>>> On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote:
>>>>> On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote:
>>>>>> On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote:
>>>>>>> Add additional pixel formats for which blending is disabling when
>>>>>> Did you mean "disabled" instead of "disabling" ?
>>> Oops.  Yes, that's exactly what I meant.
> I'll fix this locally.


Thank you very much.


>
>>>>>>> DRM_MODE_BLEND_PIXEL_NONE is set.
>>>>>>>
>>>>>>> Refactor the fourcc selection into a separate function to handle the
>>>>>>> increased number of formats.
>>>>>>>
>>>>>>> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
>>>>>>> ---
>>>>>>>    drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
>>>>>>>    1 file changed, 32 insertions(+), 17 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
>>>>>>> index 45c05d0ffc70..96241c03b60f 100644
>>>>>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
>>>>>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
>>>>>>> @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
>>>>>>>    	DRM_FORMAT_Y212,
>>>>>>>    };
>>>>>>>    
>>>>>>> +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
>>>>>>> +{
>>>>>>> +	u32 fourcc = state->format->fourcc;
>>>>>>> +
>>>>>>> +	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
>>>>>>> +		switch (fourcc) {
>>>>>>> +		case DRM_FORMAT_ARGB1555:
>>>>>>> +			fourcc = DRM_FORMAT_XRGB1555;
>>>>>>> +			break;
>>>>>>> +
>>>>>>> +		case DRM_FORMAT_ARGB4444:
>>>>>>> +			fourcc = DRM_FORMAT_XRGB4444;
>>>>>>> +			break;
>>>>>>> +
>>>>>>> +		case DRM_FORMAT_ARGB8888:
>>>>>>> +			fourcc = DRM_FORMAT_XRGB8888;
>>>>>>> +			break;
>>>>>>> +
>>>>>>> +		case DRM_FORMAT_BGRA8888:
>>>>>>> +			fourcc = DRM_FORMAT_BGRX8888;
>>>>>>> +			break;
>>>>>>> +
>>>>>>> +		case DRM_FORMAT_RGBA1010102:
>>>>>>> +			fourcc = DRM_FORMAT_RGBX1010102;
>>>>>>> +			break;
>>>>>> Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out
>>>>>> intentionally ?
>>> Yes, it was intentionally left out for the time being for the
>>> reason that you noted (i.e. DRM_FORMAT_XRGB2101010 not
>>> being handled by the DU driver).
>>>
>>>>> It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as
>>>>> DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment.
>>>>> Let's do so with a patch on top of this series.
>>> Yes, I was thinking the same thing.
>>>
>>>> Replying to myself again, the datasheet doesn't explicitly list
>>>> DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to
>>>> specify the location of the components should work fine for that format.
>>>> Is this something you would be able to test ?
>>> Unfortunately I don't have a Gen 4 system on hand to test the 2-10-10-10
>>> formats with.
>>> I will double-check with the office in Japan, but I don't think that
>>> they have one either.
>> Tomi, is this something you could test ?
> Replying to myself, this is something we could test, but let's not delay
> this series, new formats can always be added on top.

Ok, great.  Thanks very much.


Damian

>>>>> There's no need to send
>>>>> a v2, I can handle the simple change in the commit message if you let me
>>>>> know whether my comment is right or wrong.

Re: [PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

[Damian Hobson-Garcia]

Hi Laurent,


On 2023/08/14 6:46, Laurent Pinchart wrote:
> On Fri, Aug 04, 2023 at 06:52:51PM +0300, Laurent Pinchart wrote:
>> On Fri, Aug 04, 2023 at 11:49:32AM -0400, Damian Hobson-Garcia wrote:
>>> On 2023/08/03 20:06, Laurent Pinchart wrote:
>>>> On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote:
>>>>> On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote:
>>>>>> On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote:
>>>>>>> Add additional pixel formats for which blending is disabling when
>>>>>> Did you mean "disabled" instead of "disabling" ?
>>> Oops.  Yes, that's exactly what I meant.
> I'll fix this locally.


Thank you very much.


>
>>>>>>> DRM_MODE_BLEND_PIXEL_NONE is set.
>>>>>>>
>>>>>>> Refactor the fourcc selection into a separate function to handle the
>>>>>>> increased number of formats.
>>>>>>>
>>>>>>> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
>>>>>>> ---
>>>>>>>    drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
>>>>>>>    1 file changed, 32 insertions(+), 17 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
>>>>>>> index 45c05d0ffc70..96241c03b60f 100644
>>>>>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
>>>>>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
>>>>>>> @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
>>>>>>>    	DRM_FORMAT_Y212,
>>>>>>>    };
>>>>>>>    
>>>>>>> +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
>>>>>>> +{
>>>>>>> +	u32 fourcc = state->format->fourcc;
>>>>>>> +
>>>>>>> +	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
>>>>>>> +		switch (fourcc) {
>>>>>>> +		case DRM_FORMAT_ARGB1555:
>>>>>>> +			fourcc = DRM_FORMAT_XRGB1555;
>>>>>>> +			break;
>>>>>>> +
>>>>>>> +		case DRM_FORMAT_ARGB4444:
>>>>>>> +			fourcc = DRM_FORMAT_XRGB4444;
>>>>>>> +			break;
>>>>>>> +
>>>>>>> +		case DRM_FORMAT_ARGB8888:
>>>>>>> +			fourcc = DRM_FORMAT_XRGB8888;
>>>>>>> +			break;
>>>>>>> +
>>>>>>> +		case DRM_FORMAT_BGRA8888:
>>>>>>> +			fourcc = DRM_FORMAT_BGRX8888;
>>>>>>> +			break;
>>>>>>> +
>>>>>>> +		case DRM_FORMAT_RGBA1010102:
>>>>>>> +			fourcc = DRM_FORMAT_RGBX1010102;
>>>>>>> +			break;
>>>>>> Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out
>>>>>> intentionally ?
>>> Yes, it was intentionally left out for the time being for the
>>> reason that you noted (i.e. DRM_FORMAT_XRGB2101010 not
>>> being handled by the DU driver).
>>>
>>>>> It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as
>>>>> DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment.
>>>>> Let's do so with a patch on top of this series.
>>> Yes, I was thinking the same thing.
>>>
>>>> Replying to myself again, the datasheet doesn't explicitly list
>>>> DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to
>>>> specify the location of the components should work fine for that format.
>>>> Is this something you would be able to test ?
>>> Unfortunately I don't have a Gen 4 system on hand to test the 2-10-10-10
>>> formats with.
>>> I will double-check with the office in Japan, but I don't think that
>>> they have one either.
>> Tomi, is this something you could test ?
> Replying to myself, this is something we could test, but let's not delay
> this series, new formats can always be added on top.

Ok, great.  Thanks very much.


Damian

>>>>> There's no need to send
>>>>> a v2, I can handle the simple change in the commit message if you let me
>>>>> know whether my comment is right or wrong.