[PATCH v2 0/2] drm/komeda: Add layer line size support

[PATCH v2 0/2] drm/komeda: Add layer line size support

[Lowry Li (Arm Technology China)]

Hi,

From D32 every component have a line size register to indicate internal
fifo size, instead of using the global line_sz.

This serie aims at adding the layer line size support and check
accordingly on both D71 and D32 or newer.

Changes since v1:
Rebases to drm-misc-next branch.

Lowry Li (Arm Technology China) (2):
  drm/komeda: Add line size support
  drm/komeda: Adds layer horizontal input size limitation check for D71

 .../arm/display/komeda/d71/d71_component.c    | 106 ++++++++++++++++--
 .../gpu/drm/arm/display/komeda/d71/d71_regs.h |   9 +-
 .../drm/arm/display/komeda/komeda_pipeline.h  |   2 +
 .../display/komeda/komeda_pipeline_state.c    |  17 +++
 4 files changed, 119 insertions(+), 15 deletions(-)

-- 
2.17.1

[PATCH v2 0/2] drm/komeda: Add layer line size support

[Lowry Li (Arm Technology China)]

Hi,

From D32 every component have a line size register to indicate internal
fifo size, instead of using the global line_sz.

This serie aims at adding the layer line size support and check
accordingly on both D71 and D32 or newer.

Changes since v1:
Rebases to drm-misc-next branch.

Lowry Li (Arm Technology China) (2):
  drm/komeda: Add line size support
  drm/komeda: Adds layer horizontal input size limitation check for D71

 .../arm/display/komeda/d71/d71_component.c    | 106 ++++++++++++++++--
 .../gpu/drm/arm/display/komeda/d71/d71_regs.h |   9 +-
 .../drm/arm/display/komeda/komeda_pipeline.h  |   2 +
 .../display/komeda/komeda_pipeline_state.c    |  17 +++
 4 files changed, 119 insertions(+), 15 deletions(-)

-- 
2.17.1

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

[PATCH v2 1/2] drm/komeda: Add line size support

[Lowry Li (Arm Technology China)]

From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>

On D71, we are using the global line size. From D32, every
component have a line size register to indicate the fifo size.

So this patch is to set line size support and do the line size
check.

Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
---
 .../arm/display/komeda/d71/d71_component.c    | 57 ++++++++++++++++---
 .../gpu/drm/arm/display/komeda/d71/d71_regs.h |  9 +--
 .../drm/arm/display/komeda/komeda_pipeline.h  |  2 +
 .../display/komeda/komeda_pipeline_state.c    | 17 ++++++
 4 files changed, 70 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
index 7b374a3b911e..357837b9d6ed 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
@@ -106,6 +106,23 @@ static void dump_block_header(struct seq_file *sf, void __iomem *reg)
 			   i, hdr.output_ids[i]);
 }
 
+/* On D71, we are using the global line size. From D32, every component have
+ * a line size register to indicate the fifo size.
+ */
+static u32 __get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg,
+			       u32 max_default)
+{
+	if (!d71->periph_addr)
+		max_default = malidp_read32(reg, BLK_MAX_LINE_SIZE);
+
+	return max_default;
+}
+
+static u32 get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg)
+{
+	return __get_blk_line_size(d71, reg, d71->max_line_size);
+}
+
 static u32 to_rot_ctrl(u32 rot)
 {
 	u32 lr_ctrl = 0;
@@ -365,7 +382,28 @@ static int d71_layer_init(struct d71_dev *d71,
 	else
 		layer->layer_type = KOMEDA_FMT_SIMPLE_LAYER;
 
-	set_range(&layer->hsize_in, 4, d71->max_line_size);
+	if (!d71->periph_addr) {
+		/* D32 or newer product */
+		layer->line_sz = malidp_read32(reg, BLK_MAX_LINE_SIZE);
+		layer->yuv_line_sz = L_INFO_YUV_MAX_LINESZ(layer_info);
+	} else if (d71->max_line_size > 2048) {
+		/* D71 4K */
+		layer->line_sz = d71->max_line_size;
+		layer->yuv_line_sz = layer->line_sz / 2;
+	} else	{
+		/* D71 2K */
+		if (layer->layer_type == KOMEDA_FMT_RICH_LAYER) {
+			/* rich layer is 4K configuration */
+			layer->line_sz = d71->max_line_size * 2;
+			layer->yuv_line_sz = layer->line_sz / 2;
+		} else {
+			layer->line_sz = d71->max_line_size;
+			layer->yuv_line_sz = 0;
+		}
+	}
+
+	set_range(&layer->hsize_in, 4, layer->line_sz);
+
 	set_range(&layer->vsize_in, 4, d71->max_vsize);
 
 	malidp_write32(reg, LAYER_PALPHA, D71_PALPHA_DEF_MAP);
@@ -456,9 +494,11 @@ static int d71_wb_layer_init(struct d71_dev *d71,
 
 	wb_layer = to_layer(c);
 	wb_layer->layer_type = KOMEDA_FMT_WB_LAYER;
+	wb_layer->line_sz = get_blk_line_size(d71, reg);
+	wb_layer->yuv_line_sz = wb_layer->line_sz;
 
-	set_range(&wb_layer->hsize_in, D71_MIN_LINE_SIZE, d71->max_line_size);
-	set_range(&wb_layer->vsize_in, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
+	set_range(&wb_layer->hsize_in, 64, wb_layer->line_sz);
+	set_range(&wb_layer->vsize_in, 64, d71->max_vsize);
 
 	return 0;
 }
@@ -595,8 +635,8 @@ static int d71_compiz_init(struct d71_dev *d71,
 
 	compiz = to_compiz(c);
 
-	set_range(&compiz->hsize, D71_MIN_LINE_SIZE, d71->max_line_size);
-	set_range(&compiz->vsize, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
+	set_range(&compiz->hsize, 64, get_blk_line_size(d71, reg));
+	set_range(&compiz->vsize, 64, d71->max_vsize);
 
 	return 0;
 }
@@ -753,7 +793,7 @@ static int d71_scaler_init(struct d71_dev *d71,
 	}
 
 	scaler = to_scaler(c);
-	set_range(&scaler->hsize, 4, 2048);
+	set_range(&scaler->hsize, 4, __get_blk_line_size(d71, reg, 2048));
 	set_range(&scaler->vsize, 4, 4096);
 	scaler->max_downscaling = 6;
 	scaler->max_upscaling = 64;
@@ -862,7 +902,7 @@ static int d71_splitter_init(struct d71_dev *d71,
 
 	splitter = to_splitter(c);
 
-	set_range(&splitter->hsize, 4, d71->max_line_size);
+	set_range(&splitter->hsize, 4, get_blk_line_size(d71, reg));
 	set_range(&splitter->vsize, 4, d71->max_vsize);
 
 	return 0;
@@ -933,7 +973,8 @@ static int d71_merger_init(struct d71_dev *d71,
 
 	merger = to_merger(c);
 
-	set_range(&merger->hsize_merged, 4, 4032);
+	set_range(&merger->hsize_merged, 4,
+		  __get_blk_line_size(d71, reg, 4032));
 	set_range(&merger->vsize_merged, 4, 4096);
 
 	return 0;
diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
index 2d5e6d00b42c..1727dc993909 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
@@ -10,6 +10,7 @@
 /* Common block registers offset */
 #define BLK_BLOCK_INFO		0x000
 #define BLK_PIPELINE_INFO	0x004
+#define BLK_MAX_LINE_SIZE	0x008
 #define BLK_VALID_INPUT_ID0	0x020
 #define BLK_OUTPUT_ID0		0x060
 #define BLK_INPUT_ID0		0x080
@@ -321,6 +322,7 @@
 #define L_INFO_RF		BIT(0)
 #define L_INFO_CM		BIT(1)
 #define L_INFO_ABUF_SIZE(x)	(((x) >> 4) & 0x7)
+#define L_INFO_YUV_MAX_LINESZ(x)	(((x) >> 16) & 0xFFFF)
 
 /* Scaler registers */
 #define SC_COEFFTAB		0x0DC
@@ -494,13 +496,6 @@ enum d71_blk_type {
 #define D71_DEFAULT_PREPRETCH_LINE	5
 #define D71_BUS_WIDTH_16_BYTES		16
 
-#define D71_MIN_LINE_SIZE		64
-#define D71_MIN_VERTICAL_SIZE		64
-#define D71_SC_MIN_LIN_SIZE		4
-#define D71_SC_MIN_VERTICAL_SIZE	4
-#define D71_SC_MAX_LIN_SIZE		2048
-#define D71_SC_MAX_VERTICAL_SIZE	4096
-
 #define D71_SC_MAX_UPSCALING		64
 #define D71_SC_MAX_DOWNSCALING		6
 #define D71_SC_SPLIT_OVERLAP		8
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
index 910d279ae48d..92aba58ce2a5 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
@@ -227,6 +227,8 @@ struct komeda_layer {
 	/* accepted h/v input range before rotation */
 	struct malidp_range hsize_in, vsize_in;
 	u32 layer_type; /* RICH, SIMPLE or WB */
+	u32 line_sz;
+	u32 yuv_line_sz; /* maximum line size for YUV422 and YUV420 */
 	u32 supported_rots;
 	/* komeda supports layer split which splits a whole image to two parts
 	 * left and right and handle them by two individual layer processors
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
index 5526731f5a33..6df442666cfe 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
@@ -285,6 +285,7 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
 		       struct komeda_data_flow_cfg *dflow)
 {
 	u32 src_x, src_y, src_w, src_h;
+	u32 line_sz, max_line_sz;
 
 	if (!komeda_fb_is_layer_supported(kfb, layer->layer_type, dflow->rot))
 		return -EINVAL;
@@ -314,6 +315,22 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
 		return -EINVAL;
 	}
 
+	if (drm_rotation_90_or_270(dflow->rot))
+		line_sz = dflow->in_h;
+	else
+		line_sz = dflow->in_w;
+
+	if (kfb->base.format->hsub > 1)
+		max_line_sz = layer->yuv_line_sz;
+	else
+		max_line_sz = layer->line_sz;
+
+	if (line_sz > max_line_sz) {
+		DRM_DEBUG_ATOMIC("Required line_sz: %d exceeds the max size %d\n",
+				 line_sz, max_line_sz);
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
2.17.1

[PATCH v2 1/2] drm/komeda: Add line size support

[Lowry Li (Arm Technology China)]

From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>

On D71, we are using the global line size. From D32, every
component have a line size register to indicate the fifo size.

So this patch is to set line size support and do the line size
check.

Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
---
 .../arm/display/komeda/d71/d71_component.c    | 57 ++++++++++++++++---
 .../gpu/drm/arm/display/komeda/d71/d71_regs.h |  9 +--
 .../drm/arm/display/komeda/komeda_pipeline.h  |  2 +
 .../display/komeda/komeda_pipeline_state.c    | 17 ++++++
 4 files changed, 70 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
index 7b374a3b911e..357837b9d6ed 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
@@ -106,6 +106,23 @@ static void dump_block_header(struct seq_file *sf, void __iomem *reg)
 			   i, hdr.output_ids[i]);
 }
 
+/* On D71, we are using the global line size. From D32, every component have
+ * a line size register to indicate the fifo size.
+ */
+static u32 __get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg,
+			       u32 max_default)
+{
+	if (!d71->periph_addr)
+		max_default = malidp_read32(reg, BLK_MAX_LINE_SIZE);
+
+	return max_default;
+}
+
+static u32 get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg)
+{
+	return __get_blk_line_size(d71, reg, d71->max_line_size);
+}
+
 static u32 to_rot_ctrl(u32 rot)
 {
 	u32 lr_ctrl = 0;
@@ -365,7 +382,28 @@ static int d71_layer_init(struct d71_dev *d71,
 	else
 		layer->layer_type = KOMEDA_FMT_SIMPLE_LAYER;
 
-	set_range(&layer->hsize_in, 4, d71->max_line_size);
+	if (!d71->periph_addr) {
+		/* D32 or newer product */
+		layer->line_sz = malidp_read32(reg, BLK_MAX_LINE_SIZE);
+		layer->yuv_line_sz = L_INFO_YUV_MAX_LINESZ(layer_info);
+	} else if (d71->max_line_size > 2048) {
+		/* D71 4K */
+		layer->line_sz = d71->max_line_size;
+		layer->yuv_line_sz = layer->line_sz / 2;
+	} else	{
+		/* D71 2K */
+		if (layer->layer_type == KOMEDA_FMT_RICH_LAYER) {
+			/* rich layer is 4K configuration */
+			layer->line_sz = d71->max_line_size * 2;
+			layer->yuv_line_sz = layer->line_sz / 2;
+		} else {
+			layer->line_sz = d71->max_line_size;
+			layer->yuv_line_sz = 0;
+		}
+	}
+
+	set_range(&layer->hsize_in, 4, layer->line_sz);
+
 	set_range(&layer->vsize_in, 4, d71->max_vsize);
 
 	malidp_write32(reg, LAYER_PALPHA, D71_PALPHA_DEF_MAP);
@@ -456,9 +494,11 @@ static int d71_wb_layer_init(struct d71_dev *d71,
 
 	wb_layer = to_layer(c);
 	wb_layer->layer_type = KOMEDA_FMT_WB_LAYER;
+	wb_layer->line_sz = get_blk_line_size(d71, reg);
+	wb_layer->yuv_line_sz = wb_layer->line_sz;
 
-	set_range(&wb_layer->hsize_in, D71_MIN_LINE_SIZE, d71->max_line_size);
-	set_range(&wb_layer->vsize_in, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
+	set_range(&wb_layer->hsize_in, 64, wb_layer->line_sz);
+	set_range(&wb_layer->vsize_in, 64, d71->max_vsize);
 
 	return 0;
 }
@@ -595,8 +635,8 @@ static int d71_compiz_init(struct d71_dev *d71,
 
 	compiz = to_compiz(c);
 
-	set_range(&compiz->hsize, D71_MIN_LINE_SIZE, d71->max_line_size);
-	set_range(&compiz->vsize, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
+	set_range(&compiz->hsize, 64, get_blk_line_size(d71, reg));
+	set_range(&compiz->vsize, 64, d71->max_vsize);
 
 	return 0;
 }
@@ -753,7 +793,7 @@ static int d71_scaler_init(struct d71_dev *d71,
 	}
 
 	scaler = to_scaler(c);
-	set_range(&scaler->hsize, 4, 2048);
+	set_range(&scaler->hsize, 4, __get_blk_line_size(d71, reg, 2048));
 	set_range(&scaler->vsize, 4, 4096);
 	scaler->max_downscaling = 6;
 	scaler->max_upscaling = 64;
@@ -862,7 +902,7 @@ static int d71_splitter_init(struct d71_dev *d71,
 
 	splitter = to_splitter(c);
 
-	set_range(&splitter->hsize, 4, d71->max_line_size);
+	set_range(&splitter->hsize, 4, get_blk_line_size(d71, reg));
 	set_range(&splitter->vsize, 4, d71->max_vsize);
 
 	return 0;
@@ -933,7 +973,8 @@ static int d71_merger_init(struct d71_dev *d71,
 
 	merger = to_merger(c);
 
-	set_range(&merger->hsize_merged, 4, 4032);
+	set_range(&merger->hsize_merged, 4,
+		  __get_blk_line_size(d71, reg, 4032));
 	set_range(&merger->vsize_merged, 4, 4096);
 
 	return 0;
diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
index 2d5e6d00b42c..1727dc993909 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
@@ -10,6 +10,7 @@
 /* Common block registers offset */
 #define BLK_BLOCK_INFO		0x000
 #define BLK_PIPELINE_INFO	0x004
+#define BLK_MAX_LINE_SIZE	0x008
 #define BLK_VALID_INPUT_ID0	0x020
 #define BLK_OUTPUT_ID0		0x060
 #define BLK_INPUT_ID0		0x080
@@ -321,6 +322,7 @@
 #define L_INFO_RF		BIT(0)
 #define L_INFO_CM		BIT(1)
 #define L_INFO_ABUF_SIZE(x)	(((x) >> 4) & 0x7)
+#define L_INFO_YUV_MAX_LINESZ(x)	(((x) >> 16) & 0xFFFF)
 
 /* Scaler registers */
 #define SC_COEFFTAB		0x0DC
@@ -494,13 +496,6 @@ enum d71_blk_type {
 #define D71_DEFAULT_PREPRETCH_LINE	5
 #define D71_BUS_WIDTH_16_BYTES		16
 
-#define D71_MIN_LINE_SIZE		64
-#define D71_MIN_VERTICAL_SIZE		64
-#define D71_SC_MIN_LIN_SIZE		4
-#define D71_SC_MIN_VERTICAL_SIZE	4
-#define D71_SC_MAX_LIN_SIZE		2048
-#define D71_SC_MAX_VERTICAL_SIZE	4096
-
 #define D71_SC_MAX_UPSCALING		64
 #define D71_SC_MAX_DOWNSCALING		6
 #define D71_SC_SPLIT_OVERLAP		8
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
index 910d279ae48d..92aba58ce2a5 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
@@ -227,6 +227,8 @@ struct komeda_layer {
 	/* accepted h/v input range before rotation */
 	struct malidp_range hsize_in, vsize_in;
 	u32 layer_type; /* RICH, SIMPLE or WB */
+	u32 line_sz;
+	u32 yuv_line_sz; /* maximum line size for YUV422 and YUV420 */
 	u32 supported_rots;
 	/* komeda supports layer split which splits a whole image to two parts
 	 * left and right and handle them by two individual layer processors
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
index 5526731f5a33..6df442666cfe 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
@@ -285,6 +285,7 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
 		       struct komeda_data_flow_cfg *dflow)
 {
 	u32 src_x, src_y, src_w, src_h;
+	u32 line_sz, max_line_sz;
 
 	if (!komeda_fb_is_layer_supported(kfb, layer->layer_type, dflow->rot))
 		return -EINVAL;
@@ -314,6 +315,22 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
 		return -EINVAL;
 	}
 
+	if (drm_rotation_90_or_270(dflow->rot))
+		line_sz = dflow->in_h;
+	else
+		line_sz = dflow->in_w;
+
+	if (kfb->base.format->hsub > 1)
+		max_line_sz = layer->yuv_line_sz;
+	else
+		max_line_sz = layer->line_sz;
+
+	if (line_sz > max_line_sz) {
+		DRM_DEBUG_ATOMIC("Required line_sz: %d exceeds the max size %d\n",
+				 line_sz, max_line_sz);
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
2.17.1

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

Re: [PATCH v2 1/2] drm/komeda: Add line size support

[Liviu Dudau]

Hi Lowry,

On Tue, Sep 24, 2019 at 08:00:44AM +0000, Lowry Li (Arm Technology China) wrote:
> From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> 
> On D71, we are using the global line size. From D32, every
> component have a line size register to indicate the fifo size.
> 
> So this patch is to set line size support and do the line size
> check.
> 
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> ---
>  .../arm/display/komeda/d71/d71_component.c    | 57 ++++++++++++++++---
>  .../gpu/drm/arm/display/komeda/d71/d71_regs.h |  9 +--
>  .../drm/arm/display/komeda/komeda_pipeline.h  |  2 +
>  .../display/komeda/komeda_pipeline_state.c    | 17 ++++++
>  4 files changed, 70 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> index 7b374a3b911e..357837b9d6ed 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> @@ -106,6 +106,23 @@ static void dump_block_header(struct seq_file *sf, void __iomem *reg)
>  			   i, hdr.output_ids[i]);
>  }
>  
> +/* On D71, we are using the global line size. From D32, every component have
> + * a line size register to indicate the fifo size.
> + */
> +static u32 __get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg,
> +			       u32 max_default)
> +{
> +	if (!d71->periph_addr)
> +		max_default = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> +
> +	return max_default;
> +}
> +
> +static u32 get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg)
> +{
> +	return __get_blk_line_size(d71, reg, d71->max_line_size);
> +}

I know you're trying to save typing the extra parameter, but looking at the rest of
the diff I think it would look better if you get rid of get_blk_line_size() function
and use the name for the function with 3 parameters.

Otherwise, patch looks good to me.

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu

> +
>  static u32 to_rot_ctrl(u32 rot)
>  {
>  	u32 lr_ctrl = 0;
> @@ -365,7 +382,28 @@ static int d71_layer_init(struct d71_dev *d71,
>  	else
>  		layer->layer_type = KOMEDA_FMT_SIMPLE_LAYER;
>  
> -	set_range(&layer->hsize_in, 4, d71->max_line_size);
> +	if (!d71->periph_addr) {
> +		/* D32 or newer product */
> +		layer->line_sz = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> +		layer->yuv_line_sz = L_INFO_YUV_MAX_LINESZ(layer_info);
> +	} else if (d71->max_line_size > 2048) {
> +		/* D71 4K */
> +		layer->line_sz = d71->max_line_size;
> +		layer->yuv_line_sz = layer->line_sz / 2;
> +	} else	{
> +		/* D71 2K */
> +		if (layer->layer_type == KOMEDA_FMT_RICH_LAYER) {
> +			/* rich layer is 4K configuration */
> +			layer->line_sz = d71->max_line_size * 2;
> +			layer->yuv_line_sz = layer->line_sz / 2;
> +		} else {
> +			layer->line_sz = d71->max_line_size;
> +			layer->yuv_line_sz = 0;
> +		}
> +	}
> +
> +	set_range(&layer->hsize_in, 4, layer->line_sz);
> +
>  	set_range(&layer->vsize_in, 4, d71->max_vsize);
>  
>  	malidp_write32(reg, LAYER_PALPHA, D71_PALPHA_DEF_MAP);
> @@ -456,9 +494,11 @@ static int d71_wb_layer_init(struct d71_dev *d71,
>  
>  	wb_layer = to_layer(c);
>  	wb_layer->layer_type = KOMEDA_FMT_WB_LAYER;
> +	wb_layer->line_sz = get_blk_line_size(d71, reg);
> +	wb_layer->yuv_line_sz = wb_layer->line_sz;
>  
> -	set_range(&wb_layer->hsize_in, D71_MIN_LINE_SIZE, d71->max_line_size);
> -	set_range(&wb_layer->vsize_in, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> +	set_range(&wb_layer->hsize_in, 64, wb_layer->line_sz);
> +	set_range(&wb_layer->vsize_in, 64, d71->max_vsize);
>  
>  	return 0;
>  }
> @@ -595,8 +635,8 @@ static int d71_compiz_init(struct d71_dev *d71,
>  
>  	compiz = to_compiz(c);
>  
> -	set_range(&compiz->hsize, D71_MIN_LINE_SIZE, d71->max_line_size);
> -	set_range(&compiz->vsize, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> +	set_range(&compiz->hsize, 64, get_blk_line_size(d71, reg));
> +	set_range(&compiz->vsize, 64, d71->max_vsize);
>  
>  	return 0;
>  }
> @@ -753,7 +793,7 @@ static int d71_scaler_init(struct d71_dev *d71,
>  	}
>  
>  	scaler = to_scaler(c);
> -	set_range(&scaler->hsize, 4, 2048);
> +	set_range(&scaler->hsize, 4, __get_blk_line_size(d71, reg, 2048));
>  	set_range(&scaler->vsize, 4, 4096);
>  	scaler->max_downscaling = 6;
>  	scaler->max_upscaling = 64;
> @@ -862,7 +902,7 @@ static int d71_splitter_init(struct d71_dev *d71,
>  
>  	splitter = to_splitter(c);
>  
> -	set_range(&splitter->hsize, 4, d71->max_line_size);
> +	set_range(&splitter->hsize, 4, get_blk_line_size(d71, reg));
>  	set_range(&splitter->vsize, 4, d71->max_vsize);
>  
>  	return 0;
> @@ -933,7 +973,8 @@ static int d71_merger_init(struct d71_dev *d71,
>  
>  	merger = to_merger(c);
>  
> -	set_range(&merger->hsize_merged, 4, 4032);
> +	set_range(&merger->hsize_merged, 4,
> +		  __get_blk_line_size(d71, reg, 4032));
>  	set_range(&merger->vsize_merged, 4, 4096);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> index 2d5e6d00b42c..1727dc993909 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> @@ -10,6 +10,7 @@
>  /* Common block registers offset */
>  #define BLK_BLOCK_INFO		0x000
>  #define BLK_PIPELINE_INFO	0x004
> +#define BLK_MAX_LINE_SIZE	0x008
>  #define BLK_VALID_INPUT_ID0	0x020
>  #define BLK_OUTPUT_ID0		0x060
>  #define BLK_INPUT_ID0		0x080
> @@ -321,6 +322,7 @@
>  #define L_INFO_RF		BIT(0)
>  #define L_INFO_CM		BIT(1)
>  #define L_INFO_ABUF_SIZE(x)	(((x) >> 4) & 0x7)
> +#define L_INFO_YUV_MAX_LINESZ(x)	(((x) >> 16) & 0xFFFF)
>  
>  /* Scaler registers */
>  #define SC_COEFFTAB		0x0DC
> @@ -494,13 +496,6 @@ enum d71_blk_type {
>  #define D71_DEFAULT_PREPRETCH_LINE	5
>  #define D71_BUS_WIDTH_16_BYTES		16
>  
> -#define D71_MIN_LINE_SIZE		64
> -#define D71_MIN_VERTICAL_SIZE		64
> -#define D71_SC_MIN_LIN_SIZE		4
> -#define D71_SC_MIN_VERTICAL_SIZE	4
> -#define D71_SC_MAX_LIN_SIZE		2048
> -#define D71_SC_MAX_VERTICAL_SIZE	4096
> -
>  #define D71_SC_MAX_UPSCALING		64
>  #define D71_SC_MAX_DOWNSCALING		6
>  #define D71_SC_SPLIT_OVERLAP		8
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> index 910d279ae48d..92aba58ce2a5 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> @@ -227,6 +227,8 @@ struct komeda_layer {
>  	/* accepted h/v input range before rotation */
>  	struct malidp_range hsize_in, vsize_in;
>  	u32 layer_type; /* RICH, SIMPLE or WB */
> +	u32 line_sz;
> +	u32 yuv_line_sz; /* maximum line size for YUV422 and YUV420 */
>  	u32 supported_rots;
>  	/* komeda supports layer split which splits a whole image to two parts
>  	 * left and right and handle them by two individual layer processors
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> index 5526731f5a33..6df442666cfe 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> @@ -285,6 +285,7 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
>  		       struct komeda_data_flow_cfg *dflow)
>  {
>  	u32 src_x, src_y, src_w, src_h;
> +	u32 line_sz, max_line_sz;
>  
>  	if (!komeda_fb_is_layer_supported(kfb, layer->layer_type, dflow->rot))
>  		return -EINVAL;
> @@ -314,6 +315,22 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
>  		return -EINVAL;
>  	}
>  
> +	if (drm_rotation_90_or_270(dflow->rot))
> +		line_sz = dflow->in_h;
> +	else
> +		line_sz = dflow->in_w;
> +
> +	if (kfb->base.format->hsub > 1)
> +		max_line_sz = layer->yuv_line_sz;
> +	else
> +		max_line_sz = layer->line_sz;
> +
> +	if (line_sz > max_line_sz) {
> +		DRM_DEBUG_ATOMIC("Required line_sz: %d exceeds the max size %d\n",
> +				 line_sz, max_line_sz);
> +		return -EINVAL;
> +	}
> +
>  	return 0;
>  }
>  
> -- 
> 2.17.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v2 1/2] drm/komeda: Add line size support

[Liviu Dudau]

Hi Lowry,

On Tue, Sep 24, 2019 at 08:00:44AM +0000, Lowry Li (Arm Technology China) wrote:
> From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> 
> On D71, we are using the global line size. From D32, every
> component have a line size register to indicate the fifo size.
> 
> So this patch is to set line size support and do the line size
> check.
> 
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> ---
>  .../arm/display/komeda/d71/d71_component.c    | 57 ++++++++++++++++---
>  .../gpu/drm/arm/display/komeda/d71/d71_regs.h |  9 +--
>  .../drm/arm/display/komeda/komeda_pipeline.h  |  2 +
>  .../display/komeda/komeda_pipeline_state.c    | 17 ++++++
>  4 files changed, 70 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> index 7b374a3b911e..357837b9d6ed 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> @@ -106,6 +106,23 @@ static void dump_block_header(struct seq_file *sf, void __iomem *reg)
>  			   i, hdr.output_ids[i]);
>  }
>  
> +/* On D71, we are using the global line size. From D32, every component have
> + * a line size register to indicate the fifo size.
> + */
> +static u32 __get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg,
> +			       u32 max_default)
> +{
> +	if (!d71->periph_addr)
> +		max_default = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> +
> +	return max_default;
> +}
> +
> +static u32 get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg)
> +{
> +	return __get_blk_line_size(d71, reg, d71->max_line_size);
> +}

I know you're trying to save typing the extra parameter, but looking at the rest of
the diff I think it would look better if you get rid of get_blk_line_size() function
and use the name for the function with 3 parameters.

Otherwise, patch looks good to me.

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu

> +
>  static u32 to_rot_ctrl(u32 rot)
>  {
>  	u32 lr_ctrl = 0;
> @@ -365,7 +382,28 @@ static int d71_layer_init(struct d71_dev *d71,
>  	else
>  		layer->layer_type = KOMEDA_FMT_SIMPLE_LAYER;
>  
> -	set_range(&layer->hsize_in, 4, d71->max_line_size);
> +	if (!d71->periph_addr) {
> +		/* D32 or newer product */
> +		layer->line_sz = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> +		layer->yuv_line_sz = L_INFO_YUV_MAX_LINESZ(layer_info);
> +	} else if (d71->max_line_size > 2048) {
> +		/* D71 4K */
> +		layer->line_sz = d71->max_line_size;
> +		layer->yuv_line_sz = layer->line_sz / 2;
> +	} else	{
> +		/* D71 2K */
> +		if (layer->layer_type == KOMEDA_FMT_RICH_LAYER) {
> +			/* rich layer is 4K configuration */
> +			layer->line_sz = d71->max_line_size * 2;
> +			layer->yuv_line_sz = layer->line_sz / 2;
> +		} else {
> +			layer->line_sz = d71->max_line_size;
> +			layer->yuv_line_sz = 0;
> +		}
> +	}
> +
> +	set_range(&layer->hsize_in, 4, layer->line_sz);
> +
>  	set_range(&layer->vsize_in, 4, d71->max_vsize);
>  
>  	malidp_write32(reg, LAYER_PALPHA, D71_PALPHA_DEF_MAP);
> @@ -456,9 +494,11 @@ static int d71_wb_layer_init(struct d71_dev *d71,
>  
>  	wb_layer = to_layer(c);
>  	wb_layer->layer_type = KOMEDA_FMT_WB_LAYER;
> +	wb_layer->line_sz = get_blk_line_size(d71, reg);
> +	wb_layer->yuv_line_sz = wb_layer->line_sz;
>  
> -	set_range(&wb_layer->hsize_in, D71_MIN_LINE_SIZE, d71->max_line_size);
> -	set_range(&wb_layer->vsize_in, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> +	set_range(&wb_layer->hsize_in, 64, wb_layer->line_sz);
> +	set_range(&wb_layer->vsize_in, 64, d71->max_vsize);
>  
>  	return 0;
>  }
> @@ -595,8 +635,8 @@ static int d71_compiz_init(struct d71_dev *d71,
>  
>  	compiz = to_compiz(c);
>  
> -	set_range(&compiz->hsize, D71_MIN_LINE_SIZE, d71->max_line_size);
> -	set_range(&compiz->vsize, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> +	set_range(&compiz->hsize, 64, get_blk_line_size(d71, reg));
> +	set_range(&compiz->vsize, 64, d71->max_vsize);
>  
>  	return 0;
>  }
> @@ -753,7 +793,7 @@ static int d71_scaler_init(struct d71_dev *d71,
>  	}
>  
>  	scaler = to_scaler(c);
> -	set_range(&scaler->hsize, 4, 2048);
> +	set_range(&scaler->hsize, 4, __get_blk_line_size(d71, reg, 2048));
>  	set_range(&scaler->vsize, 4, 4096);
>  	scaler->max_downscaling = 6;
>  	scaler->max_upscaling = 64;
> @@ -862,7 +902,7 @@ static int d71_splitter_init(struct d71_dev *d71,
>  
>  	splitter = to_splitter(c);
>  
> -	set_range(&splitter->hsize, 4, d71->max_line_size);
> +	set_range(&splitter->hsize, 4, get_blk_line_size(d71, reg));
>  	set_range(&splitter->vsize, 4, d71->max_vsize);
>  
>  	return 0;
> @@ -933,7 +973,8 @@ static int d71_merger_init(struct d71_dev *d71,
>  
>  	merger = to_merger(c);
>  
> -	set_range(&merger->hsize_merged, 4, 4032);
> +	set_range(&merger->hsize_merged, 4,
> +		  __get_blk_line_size(d71, reg, 4032));
>  	set_range(&merger->vsize_merged, 4, 4096);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> index 2d5e6d00b42c..1727dc993909 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> @@ -10,6 +10,7 @@
>  /* Common block registers offset */
>  #define BLK_BLOCK_INFO		0x000
>  #define BLK_PIPELINE_INFO	0x004
> +#define BLK_MAX_LINE_SIZE	0x008
>  #define BLK_VALID_INPUT_ID0	0x020
>  #define BLK_OUTPUT_ID0		0x060
>  #define BLK_INPUT_ID0		0x080
> @@ -321,6 +322,7 @@
>  #define L_INFO_RF		BIT(0)
>  #define L_INFO_CM		BIT(1)
>  #define L_INFO_ABUF_SIZE(x)	(((x) >> 4) & 0x7)
> +#define L_INFO_YUV_MAX_LINESZ(x)	(((x) >> 16) & 0xFFFF)
>  
>  /* Scaler registers */
>  #define SC_COEFFTAB		0x0DC
> @@ -494,13 +496,6 @@ enum d71_blk_type {
>  #define D71_DEFAULT_PREPRETCH_LINE	5
>  #define D71_BUS_WIDTH_16_BYTES		16
>  
> -#define D71_MIN_LINE_SIZE		64
> -#define D71_MIN_VERTICAL_SIZE		64
> -#define D71_SC_MIN_LIN_SIZE		4
> -#define D71_SC_MIN_VERTICAL_SIZE	4
> -#define D71_SC_MAX_LIN_SIZE		2048
> -#define D71_SC_MAX_VERTICAL_SIZE	4096
> -
>  #define D71_SC_MAX_UPSCALING		64
>  #define D71_SC_MAX_DOWNSCALING		6
>  #define D71_SC_SPLIT_OVERLAP		8
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> index 910d279ae48d..92aba58ce2a5 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> @@ -227,6 +227,8 @@ struct komeda_layer {
>  	/* accepted h/v input range before rotation */
>  	struct malidp_range hsize_in, vsize_in;
>  	u32 layer_type; /* RICH, SIMPLE or WB */
> +	u32 line_sz;
> +	u32 yuv_line_sz; /* maximum line size for YUV422 and YUV420 */
>  	u32 supported_rots;
>  	/* komeda supports layer split which splits a whole image to two parts
>  	 * left and right and handle them by two individual layer processors
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> index 5526731f5a33..6df442666cfe 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> @@ -285,6 +285,7 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
>  		       struct komeda_data_flow_cfg *dflow)
>  {
>  	u32 src_x, src_y, src_w, src_h;
> +	u32 line_sz, max_line_sz;
>  
>  	if (!komeda_fb_is_layer_supported(kfb, layer->layer_type, dflow->rot))
>  		return -EINVAL;
> @@ -314,6 +315,22 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
>  		return -EINVAL;
>  	}
>  
> +	if (drm_rotation_90_or_270(dflow->rot))
> +		line_sz = dflow->in_h;
> +	else
> +		line_sz = dflow->in_w;
> +
> +	if (kfb->base.format->hsub > 1)
> +		max_line_sz = layer->yuv_line_sz;
> +	else
> +		max_line_sz = layer->line_sz;
> +
> +	if (line_sz > max_line_sz) {
> +		DRM_DEBUG_ATOMIC("Required line_sz: %d exceeds the max size %d\n",
> +				 line_sz, max_line_sz);
> +		return -EINVAL;
> +	}
> +
>  	return 0;
>  }
>  
> -- 
> 2.17.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v2 1/2] drm/komeda: Add line size support

[Lowry Li (Arm Technology China)]

Hi Lowry,
On Wed, Sep 25, 2019 at 10:24:58AM +0000, Liviu Dudau wrote:
> Hi Lowry,
> 
> On Tue, Sep 24, 2019 at 08:00:44AM +0000, Lowry Li (Arm Technology China) wrote:
> > From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> > 
> > On D71, we are using the global line size. From D32, every
> > component have a line size register to indicate the fifo size.
> > 
> > So this patch is to set line size support and do the line size
> > check.
> > 
> > Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> > ---
> >  .../arm/display/komeda/d71/d71_component.c    | 57 ++++++++++++++++---
> >  .../gpu/drm/arm/display/komeda/d71/d71_regs.h |  9 +--
> >  .../drm/arm/display/komeda/komeda_pipeline.h  |  2 +
> >  .../display/komeda/komeda_pipeline_state.c    | 17 ++++++
> >  4 files changed, 70 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > index 7b374a3b911e..357837b9d6ed 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > @@ -106,6 +106,23 @@ static void dump_block_header(struct seq_file *sf, void __iomem *reg)
> >  			   i, hdr.output_ids[i]);
> >  }
> >  
> > +/* On D71, we are using the global line size. From D32, every component have
> > + * a line size register to indicate the fifo size.
> > + */
> > +static u32 __get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg,
> > +			       u32 max_default)
> > +{
> > +	if (!d71->periph_addr)
> > +		max_default = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > +
> > +	return max_default;
> > +}
> > +
> > +static u32 get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg)
> > +{
> > +	return __get_blk_line_size(d71, reg, d71->max_line_size);
> > +}
> 
> I know you're trying to save typing the extra parameter, but looking at the rest of
> the diff I think it would look better if you get rid of get_blk_line_size() function
> and use the name for the function with 3 parameters.
> 
> Otherwise, patch looks good to me.
> 
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> 
> Best regards,
> Liviu
Thanks for the comments.
But considering from D32 every component have a line size register
and no need default value, so we have get_blk_line_size() without
default argument and also can save some typing and lines. That's
why we want to keep __get_blk_line_size().

> > +
> >  static u32 to_rot_ctrl(u32 rot)
> >  {
> >  	u32 lr_ctrl = 0;
> > @@ -365,7 +382,28 @@ static int d71_layer_init(struct d71_dev *d71,
> >  	else
> >  		layer->layer_type = KOMEDA_FMT_SIMPLE_LAYER;
> >  
> > -	set_range(&layer->hsize_in, 4, d71->max_line_size);
> > +	if (!d71->periph_addr) {
> > +		/* D32 or newer product */
> > +		layer->line_sz = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > +		layer->yuv_line_sz = L_INFO_YUV_MAX_LINESZ(layer_info);
> > +	} else if (d71->max_line_size > 2048) {
> > +		/* D71 4K */
> > +		layer->line_sz = d71->max_line_size;
> > +		layer->yuv_line_sz = layer->line_sz / 2;
> > +	} else	{
> > +		/* D71 2K */
> > +		if (layer->layer_type == KOMEDA_FMT_RICH_LAYER) {
> > +			/* rich layer is 4K configuration */
> > +			layer->line_sz = d71->max_line_size * 2;
> > +			layer->yuv_line_sz = layer->line_sz / 2;
> > +		} else {
> > +			layer->line_sz = d71->max_line_size;
> > +			layer->yuv_line_sz = 0;
> > +		}
> > +	}
> > +
> > +	set_range(&layer->hsize_in, 4, layer->line_sz);
> > +
> >  	set_range(&layer->vsize_in, 4, d71->max_vsize);
> >  
> >  	malidp_write32(reg, LAYER_PALPHA, D71_PALPHA_DEF_MAP);
> > @@ -456,9 +494,11 @@ static int d71_wb_layer_init(struct d71_dev *d71,
> >  
> >  	wb_layer = to_layer(c);
> >  	wb_layer->layer_type = KOMEDA_FMT_WB_LAYER;
> > +	wb_layer->line_sz = get_blk_line_size(d71, reg);
> > +	wb_layer->yuv_line_sz = wb_layer->line_sz;
> >  
> > -	set_range(&wb_layer->hsize_in, D71_MIN_LINE_SIZE, d71->max_line_size);
> > -	set_range(&wb_layer->vsize_in, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > +	set_range(&wb_layer->hsize_in, 64, wb_layer->line_sz);
> > +	set_range(&wb_layer->vsize_in, 64, d71->max_vsize);
> >  
> >  	return 0;
> >  }
> > @@ -595,8 +635,8 @@ static int d71_compiz_init(struct d71_dev *d71,
> >  
> >  	compiz = to_compiz(c);
> >  
> > -	set_range(&compiz->hsize, D71_MIN_LINE_SIZE, d71->max_line_size);
> > -	set_range(&compiz->vsize, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > +	set_range(&compiz->hsize, 64, get_blk_line_size(d71, reg));
> > +	set_range(&compiz->vsize, 64, d71->max_vsize);
> >  
> >  	return 0;
> >  }
> > @@ -753,7 +793,7 @@ static int d71_scaler_init(struct d71_dev *d71,
> >  	}
> >  
> >  	scaler = to_scaler(c);
> > -	set_range(&scaler->hsize, 4, 2048);
> > +	set_range(&scaler->hsize, 4, __get_blk_line_size(d71, reg, 2048));
> >  	set_range(&scaler->vsize, 4, 4096);
> >  	scaler->max_downscaling = 6;
> >  	scaler->max_upscaling = 64;
> > @@ -862,7 +902,7 @@ static int d71_splitter_init(struct d71_dev *d71,
> >  
> >  	splitter = to_splitter(c);
> >  
> > -	set_range(&splitter->hsize, 4, d71->max_line_size);
> > +	set_range(&splitter->hsize, 4, get_blk_line_size(d71, reg));
> >  	set_range(&splitter->vsize, 4, d71->max_vsize);
> >  
> >  	return 0;
> > @@ -933,7 +973,8 @@ static int d71_merger_init(struct d71_dev *d71,
> >  
> >  	merger = to_merger(c);
> >  
> > -	set_range(&merger->hsize_merged, 4, 4032);
> > +	set_range(&merger->hsize_merged, 4,
> > +		  __get_blk_line_size(d71, reg, 4032));
> >  	set_range(&merger->vsize_merged, 4, 4096);
> >  
> >  	return 0;
> > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > index 2d5e6d00b42c..1727dc993909 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > @@ -10,6 +10,7 @@
> >  /* Common block registers offset */
> >  #define BLK_BLOCK_INFO		0x000
> >  #define BLK_PIPELINE_INFO	0x004
> > +#define BLK_MAX_LINE_SIZE	0x008
> >  #define BLK_VALID_INPUT_ID0	0x020
> >  #define BLK_OUTPUT_ID0		0x060
> >  #define BLK_INPUT_ID0		0x080
> > @@ -321,6 +322,7 @@
> >  #define L_INFO_RF		BIT(0)
> >  #define L_INFO_CM		BIT(1)
> >  #define L_INFO_ABUF_SIZE(x)	(((x) >> 4) & 0x7)
> > +#define L_INFO_YUV_MAX_LINESZ(x)	(((x) >> 16) & 0xFFFF)
> >  
> >  /* Scaler registers */
> >  #define SC_COEFFTAB		0x0DC
> > @@ -494,13 +496,6 @@ enum d71_blk_type {
> >  #define D71_DEFAULT_PREPRETCH_LINE	5
> >  #define D71_BUS_WIDTH_16_BYTES		16
> >  
> > -#define D71_MIN_LINE_SIZE		64
> > -#define D71_MIN_VERTICAL_SIZE		64
> > -#define D71_SC_MIN_LIN_SIZE		4
> > -#define D71_SC_MIN_VERTICAL_SIZE	4
> > -#define D71_SC_MAX_LIN_SIZE		2048
> > -#define D71_SC_MAX_VERTICAL_SIZE	4096
> > -
> >  #define D71_SC_MAX_UPSCALING		64
> >  #define D71_SC_MAX_DOWNSCALING		6
> >  #define D71_SC_SPLIT_OVERLAP		8
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > index 910d279ae48d..92aba58ce2a5 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > @@ -227,6 +227,8 @@ struct komeda_layer {
> >  	/* accepted h/v input range before rotation */
> >  	struct malidp_range hsize_in, vsize_in;
> >  	u32 layer_type; /* RICH, SIMPLE or WB */
> > +	u32 line_sz;
> > +	u32 yuv_line_sz; /* maximum line size for YUV422 and YUV420 */
> >  	u32 supported_rots;
> >  	/* komeda supports layer split which splits a whole image to two parts
> >  	 * left and right and handle them by two individual layer processors
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > index 5526731f5a33..6df442666cfe 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > @@ -285,6 +285,7 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> >  		       struct komeda_data_flow_cfg *dflow)
> >  {
> >  	u32 src_x, src_y, src_w, src_h;
> > +	u32 line_sz, max_line_sz;
> >  
> >  	if (!komeda_fb_is_layer_supported(kfb, layer->layer_type, dflow->rot))
> >  		return -EINVAL;
> > @@ -314,6 +315,22 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> >  		return -EINVAL;
> >  	}
> >  
> > +	if (drm_rotation_90_or_270(dflow->rot))
> > +		line_sz = dflow->in_h;
> > +	else
> > +		line_sz = dflow->in_w;
> > +
> > +	if (kfb->base.format->hsub > 1)
> > +		max_line_sz = layer->yuv_line_sz;
> > +	else
> > +		max_line_sz = layer->line_sz;
> > +
> > +	if (line_sz > max_line_sz) {
> > +		DRM_DEBUG_ATOMIC("Required line_sz: %d exceeds the max size %d\n",
> > +				 line_sz, max_line_sz);
> > +		return -EINVAL;
> > +	}
> > +
> >  	return 0;
> >  }
> >  
> > -- 
> > 2.17.1
> > 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

Re: [PATCH v2 1/2] drm/komeda: Add line size support

[Lowry Li (Arm Technology China)]

Hi Lowry,
On Wed, Sep 25, 2019 at 10:24:58AM +0000, Liviu Dudau wrote:
> Hi Lowry,
> 
> On Tue, Sep 24, 2019 at 08:00:44AM +0000, Lowry Li (Arm Technology China) wrote:
> > From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> > 
> > On D71, we are using the global line size. From D32, every
> > component have a line size register to indicate the fifo size.
> > 
> > So this patch is to set line size support and do the line size
> > check.
> > 
> > Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> > ---
> >  .../arm/display/komeda/d71/d71_component.c    | 57 ++++++++++++++++---
> >  .../gpu/drm/arm/display/komeda/d71/d71_regs.h |  9 +--
> >  .../drm/arm/display/komeda/komeda_pipeline.h  |  2 +
> >  .../display/komeda/komeda_pipeline_state.c    | 17 ++++++
> >  4 files changed, 70 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > index 7b374a3b911e..357837b9d6ed 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > @@ -106,6 +106,23 @@ static void dump_block_header(struct seq_file *sf, void __iomem *reg)
> >  			   i, hdr.output_ids[i]);
> >  }
> >  
> > +/* On D71, we are using the global line size. From D32, every component have
> > + * a line size register to indicate the fifo size.
> > + */
> > +static u32 __get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg,
> > +			       u32 max_default)
> > +{
> > +	if (!d71->periph_addr)
> > +		max_default = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > +
> > +	return max_default;
> > +}
> > +
> > +static u32 get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg)
> > +{
> > +	return __get_blk_line_size(d71, reg, d71->max_line_size);
> > +}
> 
> I know you're trying to save typing the extra parameter, but looking at the rest of
> the diff I think it would look better if you get rid of get_blk_line_size() function
> and use the name for the function with 3 parameters.
> 
> Otherwise, patch looks good to me.
> 
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> 
> Best regards,
> Liviu
Thanks for the comments.
But considering from D32 every component have a line size register
and no need default value, so we have get_blk_line_size() without
default argument and also can save some typing and lines. That's
why we want to keep __get_blk_line_size().

> > +
> >  static u32 to_rot_ctrl(u32 rot)
> >  {
> >  	u32 lr_ctrl = 0;
> > @@ -365,7 +382,28 @@ static int d71_layer_init(struct d71_dev *d71,
> >  	else
> >  		layer->layer_type = KOMEDA_FMT_SIMPLE_LAYER;
> >  
> > -	set_range(&layer->hsize_in, 4, d71->max_line_size);
> > +	if (!d71->periph_addr) {
> > +		/* D32 or newer product */
> > +		layer->line_sz = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > +		layer->yuv_line_sz = L_INFO_YUV_MAX_LINESZ(layer_info);
> > +	} else if (d71->max_line_size > 2048) {
> > +		/* D71 4K */
> > +		layer->line_sz = d71->max_line_size;
> > +		layer->yuv_line_sz = layer->line_sz / 2;
> > +	} else	{
> > +		/* D71 2K */
> > +		if (layer->layer_type == KOMEDA_FMT_RICH_LAYER) {
> > +			/* rich layer is 4K configuration */
> > +			layer->line_sz = d71->max_line_size * 2;
> > +			layer->yuv_line_sz = layer->line_sz / 2;
> > +		} else {
> > +			layer->line_sz = d71->max_line_size;
> > +			layer->yuv_line_sz = 0;
> > +		}
> > +	}
> > +
> > +	set_range(&layer->hsize_in, 4, layer->line_sz);
> > +
> >  	set_range(&layer->vsize_in, 4, d71->max_vsize);
> >  
> >  	malidp_write32(reg, LAYER_PALPHA, D71_PALPHA_DEF_MAP);
> > @@ -456,9 +494,11 @@ static int d71_wb_layer_init(struct d71_dev *d71,
> >  
> >  	wb_layer = to_layer(c);
> >  	wb_layer->layer_type = KOMEDA_FMT_WB_LAYER;
> > +	wb_layer->line_sz = get_blk_line_size(d71, reg);
> > +	wb_layer->yuv_line_sz = wb_layer->line_sz;
> >  
> > -	set_range(&wb_layer->hsize_in, D71_MIN_LINE_SIZE, d71->max_line_size);
> > -	set_range(&wb_layer->vsize_in, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > +	set_range(&wb_layer->hsize_in, 64, wb_layer->line_sz);
> > +	set_range(&wb_layer->vsize_in, 64, d71->max_vsize);
> >  
> >  	return 0;
> >  }
> > @@ -595,8 +635,8 @@ static int d71_compiz_init(struct d71_dev *d71,
> >  
> >  	compiz = to_compiz(c);
> >  
> > -	set_range(&compiz->hsize, D71_MIN_LINE_SIZE, d71->max_line_size);
> > -	set_range(&compiz->vsize, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > +	set_range(&compiz->hsize, 64, get_blk_line_size(d71, reg));
> > +	set_range(&compiz->vsize, 64, d71->max_vsize);
> >  
> >  	return 0;
> >  }
> > @@ -753,7 +793,7 @@ static int d71_scaler_init(struct d71_dev *d71,
> >  	}
> >  
> >  	scaler = to_scaler(c);
> > -	set_range(&scaler->hsize, 4, 2048);
> > +	set_range(&scaler->hsize, 4, __get_blk_line_size(d71, reg, 2048));
> >  	set_range(&scaler->vsize, 4, 4096);
> >  	scaler->max_downscaling = 6;
> >  	scaler->max_upscaling = 64;
> > @@ -862,7 +902,7 @@ static int d71_splitter_init(struct d71_dev *d71,
> >  
> >  	splitter = to_splitter(c);
> >  
> > -	set_range(&splitter->hsize, 4, d71->max_line_size);
> > +	set_range(&splitter->hsize, 4, get_blk_line_size(d71, reg));
> >  	set_range(&splitter->vsize, 4, d71->max_vsize);
> >  
> >  	return 0;
> > @@ -933,7 +973,8 @@ static int d71_merger_init(struct d71_dev *d71,
> >  
> >  	merger = to_merger(c);
> >  
> > -	set_range(&merger->hsize_merged, 4, 4032);
> > +	set_range(&merger->hsize_merged, 4,
> > +		  __get_blk_line_size(d71, reg, 4032));
> >  	set_range(&merger->vsize_merged, 4, 4096);
> >  
> >  	return 0;
> > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > index 2d5e6d00b42c..1727dc993909 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > @@ -10,6 +10,7 @@
> >  /* Common block registers offset */
> >  #define BLK_BLOCK_INFO		0x000
> >  #define BLK_PIPELINE_INFO	0x004
> > +#define BLK_MAX_LINE_SIZE	0x008
> >  #define BLK_VALID_INPUT_ID0	0x020
> >  #define BLK_OUTPUT_ID0		0x060
> >  #define BLK_INPUT_ID0		0x080
> > @@ -321,6 +322,7 @@
> >  #define L_INFO_RF		BIT(0)
> >  #define L_INFO_CM		BIT(1)
> >  #define L_INFO_ABUF_SIZE(x)	(((x) >> 4) & 0x7)
> > +#define L_INFO_YUV_MAX_LINESZ(x)	(((x) >> 16) & 0xFFFF)
> >  
> >  /* Scaler registers */
> >  #define SC_COEFFTAB		0x0DC
> > @@ -494,13 +496,6 @@ enum d71_blk_type {
> >  #define D71_DEFAULT_PREPRETCH_LINE	5
> >  #define D71_BUS_WIDTH_16_BYTES		16
> >  
> > -#define D71_MIN_LINE_SIZE		64
> > -#define D71_MIN_VERTICAL_SIZE		64
> > -#define D71_SC_MIN_LIN_SIZE		4
> > -#define D71_SC_MIN_VERTICAL_SIZE	4
> > -#define D71_SC_MAX_LIN_SIZE		2048
> > -#define D71_SC_MAX_VERTICAL_SIZE	4096
> > -
> >  #define D71_SC_MAX_UPSCALING		64
> >  #define D71_SC_MAX_DOWNSCALING		6
> >  #define D71_SC_SPLIT_OVERLAP		8
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > index 910d279ae48d..92aba58ce2a5 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > @@ -227,6 +227,8 @@ struct komeda_layer {
> >  	/* accepted h/v input range before rotation */
> >  	struct malidp_range hsize_in, vsize_in;
> >  	u32 layer_type; /* RICH, SIMPLE or WB */
> > +	u32 line_sz;
> > +	u32 yuv_line_sz; /* maximum line size for YUV422 and YUV420 */
> >  	u32 supported_rots;
> >  	/* komeda supports layer split which splits a whole image to two parts
> >  	 * left and right and handle them by two individual layer processors
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > index 5526731f5a33..6df442666cfe 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > @@ -285,6 +285,7 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> >  		       struct komeda_data_flow_cfg *dflow)
> >  {
> >  	u32 src_x, src_y, src_w, src_h;
> > +	u32 line_sz, max_line_sz;
> >  
> >  	if (!komeda_fb_is_layer_supported(kfb, layer->layer_type, dflow->rot))
> >  		return -EINVAL;
> > @@ -314,6 +315,22 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> >  		return -EINVAL;
> >  	}
> >  
> > +	if (drm_rotation_90_or_270(dflow->rot))
> > +		line_sz = dflow->in_h;
> > +	else
> > +		line_sz = dflow->in_w;
> > +
> > +	if (kfb->base.format->hsub > 1)
> > +		max_line_sz = layer->yuv_line_sz;
> > +	else
> > +		max_line_sz = layer->line_sz;
> > +
> > +	if (line_sz > max_line_sz) {
> > +		DRM_DEBUG_ATOMIC("Required line_sz: %d exceeds the max size %d\n",
> > +				 line_sz, max_line_sz);
> > +		return -EINVAL;
> > +	}
> > +
> >  	return 0;
> >  }
> >  
> > -- 
> > 2.17.1
> > 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

Re: [PATCH v2 1/2] drm/komeda: Add line size support

[Liviu Dudau]

On Thu, Sep 26, 2019 at 10:00:22AM +0000, Lowry Li (Arm Technology China) wrote:
> Hi Lowry,
> On Wed, Sep 25, 2019 at 10:24:58AM +0000, Liviu Dudau wrote:
> > Hi Lowry,
> > 
> > On Tue, Sep 24, 2019 at 08:00:44AM +0000, Lowry Li (Arm Technology China) wrote:
> > > From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> > > 
> > > On D71, we are using the global line size. From D32, every
> > > component have a line size register to indicate the fifo size.
> > > 
> > > So this patch is to set line size support and do the line size
> > > check.
> > > 
> > > Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> > > ---
> > >  .../arm/display/komeda/d71/d71_component.c    | 57 ++++++++++++++++---
> > >  .../gpu/drm/arm/display/komeda/d71/d71_regs.h |  9 +--
> > >  .../drm/arm/display/komeda/komeda_pipeline.h  |  2 +
> > >  .../display/komeda/komeda_pipeline_state.c    | 17 ++++++
> > >  4 files changed, 70 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > index 7b374a3b911e..357837b9d6ed 100644
> > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > @@ -106,6 +106,23 @@ static void dump_block_header(struct seq_file *sf, void __iomem *reg)
> > >  			   i, hdr.output_ids[i]);
> > >  }
> > >  
> > > +/* On D71, we are using the global line size. From D32, every component have
> > > + * a line size register to indicate the fifo size.
> > > + */
> > > +static u32 __get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg,
> > > +			       u32 max_default)
> > > +{
> > > +	if (!d71->periph_addr)
> > > +		max_default = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > +
> > > +	return max_default;
> > > +}
> > > +
> > > +static u32 get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg)
> > > +{
> > > +	return __get_blk_line_size(d71, reg, d71->max_line_size);
> > > +}
> > 
> > I know you're trying to save typing the extra parameter, but looking at the rest of
> > the diff I think it would look better if you get rid of get_blk_line_size() function
> > and use the name for the function with 3 parameters.
> > 
> > Otherwise, patch looks good to me.
> > 
> > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> > 
> > Best regards,
> > Liviu
> Thanks for the comments.
> But considering from D32 every component have a line size register
> and no need default value, so we have get_blk_line_size() without
> default argument and also can save some typing and lines. That's
> why we want to keep __get_blk_line_size().

I was suggesting to remove get_blk_line_size and only use __get_blk_line_size() with
explicit use of d71->max_line_size where it makes sense.

Best regards,
Liviu

> 
> > > +
> > >  static u32 to_rot_ctrl(u32 rot)
> > >  {
> > >  	u32 lr_ctrl = 0;
> > > @@ -365,7 +382,28 @@ static int d71_layer_init(struct d71_dev *d71,
> > >  	else
> > >  		layer->layer_type = KOMEDA_FMT_SIMPLE_LAYER;
> > >  
> > > -	set_range(&layer->hsize_in, 4, d71->max_line_size);
> > > +	if (!d71->periph_addr) {
> > > +		/* D32 or newer product */
> > > +		layer->line_sz = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > +		layer->yuv_line_sz = L_INFO_YUV_MAX_LINESZ(layer_info);
> > > +	} else if (d71->max_line_size > 2048) {
> > > +		/* D71 4K */
> > > +		layer->line_sz = d71->max_line_size;
> > > +		layer->yuv_line_sz = layer->line_sz / 2;
> > > +	} else	{
> > > +		/* D71 2K */
> > > +		if (layer->layer_type == KOMEDA_FMT_RICH_LAYER) {
> > > +			/* rich layer is 4K configuration */
> > > +			layer->line_sz = d71->max_line_size * 2;
> > > +			layer->yuv_line_sz = layer->line_sz / 2;
> > > +		} else {
> > > +			layer->line_sz = d71->max_line_size;
> > > +			layer->yuv_line_sz = 0;
> > > +		}
> > > +	}
> > > +
> > > +	set_range(&layer->hsize_in, 4, layer->line_sz);
> > > +
> > >  	set_range(&layer->vsize_in, 4, d71->max_vsize);
> > >  
> > >  	malidp_write32(reg, LAYER_PALPHA, D71_PALPHA_DEF_MAP);
> > > @@ -456,9 +494,11 @@ static int d71_wb_layer_init(struct d71_dev *d71,
> > >  
> > >  	wb_layer = to_layer(c);
> > >  	wb_layer->layer_type = KOMEDA_FMT_WB_LAYER;
> > > +	wb_layer->line_sz = get_blk_line_size(d71, reg);
> > > +	wb_layer->yuv_line_sz = wb_layer->line_sz;
> > >  
> > > -	set_range(&wb_layer->hsize_in, D71_MIN_LINE_SIZE, d71->max_line_size);
> > > -	set_range(&wb_layer->vsize_in, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > > +	set_range(&wb_layer->hsize_in, 64, wb_layer->line_sz);
> > > +	set_range(&wb_layer->vsize_in, 64, d71->max_vsize);
> > >  
> > >  	return 0;
> > >  }
> > > @@ -595,8 +635,8 @@ static int d71_compiz_init(struct d71_dev *d71,
> > >  
> > >  	compiz = to_compiz(c);
> > >  
> > > -	set_range(&compiz->hsize, D71_MIN_LINE_SIZE, d71->max_line_size);
> > > -	set_range(&compiz->vsize, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > > +	set_range(&compiz->hsize, 64, get_blk_line_size(d71, reg));
> > > +	set_range(&compiz->vsize, 64, d71->max_vsize);
> > >  
> > >  	return 0;
> > >  }
> > > @@ -753,7 +793,7 @@ static int d71_scaler_init(struct d71_dev *d71,
> > >  	}
> > >  
> > >  	scaler = to_scaler(c);
> > > -	set_range(&scaler->hsize, 4, 2048);
> > > +	set_range(&scaler->hsize, 4, __get_blk_line_size(d71, reg, 2048));
> > >  	set_range(&scaler->vsize, 4, 4096);
> > >  	scaler->max_downscaling = 6;
> > >  	scaler->max_upscaling = 64;
> > > @@ -862,7 +902,7 @@ static int d71_splitter_init(struct d71_dev *d71,
> > >  
> > >  	splitter = to_splitter(c);
> > >  
> > > -	set_range(&splitter->hsize, 4, d71->max_line_size);
> > > +	set_range(&splitter->hsize, 4, get_blk_line_size(d71, reg));
> > >  	set_range(&splitter->vsize, 4, d71->max_vsize);
> > >  
> > >  	return 0;
> > > @@ -933,7 +973,8 @@ static int d71_merger_init(struct d71_dev *d71,
> > >  
> > >  	merger = to_merger(c);
> > >  
> > > -	set_range(&merger->hsize_merged, 4, 4032);
> > > +	set_range(&merger->hsize_merged, 4,
> > > +		  __get_blk_line_size(d71, reg, 4032));
> > >  	set_range(&merger->vsize_merged, 4, 4096);
> > >  
> > >  	return 0;
> > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > index 2d5e6d00b42c..1727dc993909 100644
> > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > @@ -10,6 +10,7 @@
> > >  /* Common block registers offset */
> > >  #define BLK_BLOCK_INFO		0x000
> > >  #define BLK_PIPELINE_INFO	0x004
> > > +#define BLK_MAX_LINE_SIZE	0x008
> > >  #define BLK_VALID_INPUT_ID0	0x020
> > >  #define BLK_OUTPUT_ID0		0x060
> > >  #define BLK_INPUT_ID0		0x080
> > > @@ -321,6 +322,7 @@
> > >  #define L_INFO_RF		BIT(0)
> > >  #define L_INFO_CM		BIT(1)
> > >  #define L_INFO_ABUF_SIZE(x)	(((x) >> 4) & 0x7)
> > > +#define L_INFO_YUV_MAX_LINESZ(x)	(((x) >> 16) & 0xFFFF)
> > >  
> > >  /* Scaler registers */
> > >  #define SC_COEFFTAB		0x0DC
> > > @@ -494,13 +496,6 @@ enum d71_blk_type {
> > >  #define D71_DEFAULT_PREPRETCH_LINE	5
> > >  #define D71_BUS_WIDTH_16_BYTES		16
> > >  
> > > -#define D71_MIN_LINE_SIZE		64
> > > -#define D71_MIN_VERTICAL_SIZE		64
> > > -#define D71_SC_MIN_LIN_SIZE		4
> > > -#define D71_SC_MIN_VERTICAL_SIZE	4
> > > -#define D71_SC_MAX_LIN_SIZE		2048
> > > -#define D71_SC_MAX_VERTICAL_SIZE	4096
> > > -
> > >  #define D71_SC_MAX_UPSCALING		64
> > >  #define D71_SC_MAX_DOWNSCALING		6
> > >  #define D71_SC_SPLIT_OVERLAP		8
> > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > index 910d279ae48d..92aba58ce2a5 100644
> > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > @@ -227,6 +227,8 @@ struct komeda_layer {
> > >  	/* accepted h/v input range before rotation */
> > >  	struct malidp_range hsize_in, vsize_in;
> > >  	u32 layer_type; /* RICH, SIMPLE or WB */
> > > +	u32 line_sz;
> > > +	u32 yuv_line_sz; /* maximum line size for YUV422 and YUV420 */
> > >  	u32 supported_rots;
> > >  	/* komeda supports layer split which splits a whole image to two parts
> > >  	 * left and right and handle them by two individual layer processors
> > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > index 5526731f5a33..6df442666cfe 100644
> > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > @@ -285,6 +285,7 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> > >  		       struct komeda_data_flow_cfg *dflow)
> > >  {
> > >  	u32 src_x, src_y, src_w, src_h;
> > > +	u32 line_sz, max_line_sz;
> > >  
> > >  	if (!komeda_fb_is_layer_supported(kfb, layer->layer_type, dflow->rot))
> > >  		return -EINVAL;
> > > @@ -314,6 +315,22 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> > >  		return -EINVAL;
> > >  	}
> > >  
> > > +	if (drm_rotation_90_or_270(dflow->rot))
> > > +		line_sz = dflow->in_h;
> > > +	else
> > > +		line_sz = dflow->in_w;
> > > +
> > > +	if (kfb->base.format->hsub > 1)
> > > +		max_line_sz = layer->yuv_line_sz;
> > > +	else
> > > +		max_line_sz = layer->line_sz;
> > > +
> > > +	if (line_sz > max_line_sz) {
> > > +		DRM_DEBUG_ATOMIC("Required line_sz: %d exceeds the max size %d\n",
> > > +				 line_sz, max_line_sz);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > >  	return 0;
> > >  }
> > >  
> > > -- 
> > > 2.17.1
> > > 
> > 
> > -- 
> > ====================
> > | I would like to |
> > | fix the world,  |
> > | but they're not |
> > | giving me the   |
> >  \ source code!  /
> >   ---------------
> >     ¯\_(ツ)_/¯

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v2 1/2] drm/komeda: Add line size support

[Liviu Dudau]

On Thu, Sep 26, 2019 at 10:00:22AM +0000, Lowry Li (Arm Technology China) wrote:
> Hi Lowry,
> On Wed, Sep 25, 2019 at 10:24:58AM +0000, Liviu Dudau wrote:
> > Hi Lowry,
> > 
> > On Tue, Sep 24, 2019 at 08:00:44AM +0000, Lowry Li (Arm Technology China) wrote:
> > > From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> > > 
> > > On D71, we are using the global line size. From D32, every
> > > component have a line size register to indicate the fifo size.
> > > 
> > > So this patch is to set line size support and do the line size
> > > check.
> > > 
> > > Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> > > ---
> > >  .../arm/display/komeda/d71/d71_component.c    | 57 ++++++++++++++++---
> > >  .../gpu/drm/arm/display/komeda/d71/d71_regs.h |  9 +--
> > >  .../drm/arm/display/komeda/komeda_pipeline.h  |  2 +
> > >  .../display/komeda/komeda_pipeline_state.c    | 17 ++++++
> > >  4 files changed, 70 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > index 7b374a3b911e..357837b9d6ed 100644
> > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > @@ -106,6 +106,23 @@ static void dump_block_header(struct seq_file *sf, void __iomem *reg)
> > >  			   i, hdr.output_ids[i]);
> > >  }
> > >  
> > > +/* On D71, we are using the global line size. From D32, every component have
> > > + * a line size register to indicate the fifo size.
> > > + */
> > > +static u32 __get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg,
> > > +			       u32 max_default)
> > > +{
> > > +	if (!d71->periph_addr)
> > > +		max_default = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > +
> > > +	return max_default;
> > > +}
> > > +
> > > +static u32 get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg)
> > > +{
> > > +	return __get_blk_line_size(d71, reg, d71->max_line_size);
> > > +}
> > 
> > I know you're trying to save typing the extra parameter, but looking at the rest of
> > the diff I think it would look better if you get rid of get_blk_line_size() function
> > and use the name for the function with 3 parameters.
> > 
> > Otherwise, patch looks good to me.
> > 
> > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> > 
> > Best regards,
> > Liviu
> Thanks for the comments.
> But considering from D32 every component have a line size register
> and no need default value, so we have get_blk_line_size() without
> default argument and also can save some typing and lines. That's
> why we want to keep __get_blk_line_size().

I was suggesting to remove get_blk_line_size and only use __get_blk_line_size() with
explicit use of d71->max_line_size where it makes sense.

Best regards,
Liviu

> 
> > > +
> > >  static u32 to_rot_ctrl(u32 rot)
> > >  {
> > >  	u32 lr_ctrl = 0;
> > > @@ -365,7 +382,28 @@ static int d71_layer_init(struct d71_dev *d71,
> > >  	else
> > >  		layer->layer_type = KOMEDA_FMT_SIMPLE_LAYER;
> > >  
> > > -	set_range(&layer->hsize_in, 4, d71->max_line_size);
> > > +	if (!d71->periph_addr) {
> > > +		/* D32 or newer product */
> > > +		layer->line_sz = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > +		layer->yuv_line_sz = L_INFO_YUV_MAX_LINESZ(layer_info);
> > > +	} else if (d71->max_line_size > 2048) {
> > > +		/* D71 4K */
> > > +		layer->line_sz = d71->max_line_size;
> > > +		layer->yuv_line_sz = layer->line_sz / 2;
> > > +	} else	{
> > > +		/* D71 2K */
> > > +		if (layer->layer_type == KOMEDA_FMT_RICH_LAYER) {
> > > +			/* rich layer is 4K configuration */
> > > +			layer->line_sz = d71->max_line_size * 2;
> > > +			layer->yuv_line_sz = layer->line_sz / 2;
> > > +		} else {
> > > +			layer->line_sz = d71->max_line_size;
> > > +			layer->yuv_line_sz = 0;
> > > +		}
> > > +	}
> > > +
> > > +	set_range(&layer->hsize_in, 4, layer->line_sz);
> > > +
> > >  	set_range(&layer->vsize_in, 4, d71->max_vsize);
> > >  
> > >  	malidp_write32(reg, LAYER_PALPHA, D71_PALPHA_DEF_MAP);
> > > @@ -456,9 +494,11 @@ static int d71_wb_layer_init(struct d71_dev *d71,
> > >  
> > >  	wb_layer = to_layer(c);
> > >  	wb_layer->layer_type = KOMEDA_FMT_WB_LAYER;
> > > +	wb_layer->line_sz = get_blk_line_size(d71, reg);
> > > +	wb_layer->yuv_line_sz = wb_layer->line_sz;
> > >  
> > > -	set_range(&wb_layer->hsize_in, D71_MIN_LINE_SIZE, d71->max_line_size);
> > > -	set_range(&wb_layer->vsize_in, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > > +	set_range(&wb_layer->hsize_in, 64, wb_layer->line_sz);
> > > +	set_range(&wb_layer->vsize_in, 64, d71->max_vsize);
> > >  
> > >  	return 0;
> > >  }
> > > @@ -595,8 +635,8 @@ static int d71_compiz_init(struct d71_dev *d71,
> > >  
> > >  	compiz = to_compiz(c);
> > >  
> > > -	set_range(&compiz->hsize, D71_MIN_LINE_SIZE, d71->max_line_size);
> > > -	set_range(&compiz->vsize, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > > +	set_range(&compiz->hsize, 64, get_blk_line_size(d71, reg));
> > > +	set_range(&compiz->vsize, 64, d71->max_vsize);
> > >  
> > >  	return 0;
> > >  }
> > > @@ -753,7 +793,7 @@ static int d71_scaler_init(struct d71_dev *d71,
> > >  	}
> > >  
> > >  	scaler = to_scaler(c);
> > > -	set_range(&scaler->hsize, 4, 2048);
> > > +	set_range(&scaler->hsize, 4, __get_blk_line_size(d71, reg, 2048));
> > >  	set_range(&scaler->vsize, 4, 4096);
> > >  	scaler->max_downscaling = 6;
> > >  	scaler->max_upscaling = 64;
> > > @@ -862,7 +902,7 @@ static int d71_splitter_init(struct d71_dev *d71,
> > >  
> > >  	splitter = to_splitter(c);
> > >  
> > > -	set_range(&splitter->hsize, 4, d71->max_line_size);
> > > +	set_range(&splitter->hsize, 4, get_blk_line_size(d71, reg));
> > >  	set_range(&splitter->vsize, 4, d71->max_vsize);
> > >  
> > >  	return 0;
> > > @@ -933,7 +973,8 @@ static int d71_merger_init(struct d71_dev *d71,
> > >  
> > >  	merger = to_merger(c);
> > >  
> > > -	set_range(&merger->hsize_merged, 4, 4032);
> > > +	set_range(&merger->hsize_merged, 4,
> > > +		  __get_blk_line_size(d71, reg, 4032));
> > >  	set_range(&merger->vsize_merged, 4, 4096);
> > >  
> > >  	return 0;
> > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > index 2d5e6d00b42c..1727dc993909 100644
> > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > @@ -10,6 +10,7 @@
> > >  /* Common block registers offset */
> > >  #define BLK_BLOCK_INFO		0x000
> > >  #define BLK_PIPELINE_INFO	0x004
> > > +#define BLK_MAX_LINE_SIZE	0x008
> > >  #define BLK_VALID_INPUT_ID0	0x020
> > >  #define BLK_OUTPUT_ID0		0x060
> > >  #define BLK_INPUT_ID0		0x080
> > > @@ -321,6 +322,7 @@
> > >  #define L_INFO_RF		BIT(0)
> > >  #define L_INFO_CM		BIT(1)
> > >  #define L_INFO_ABUF_SIZE(x)	(((x) >> 4) & 0x7)
> > > +#define L_INFO_YUV_MAX_LINESZ(x)	(((x) >> 16) & 0xFFFF)
> > >  
> > >  /* Scaler registers */
> > >  #define SC_COEFFTAB		0x0DC
> > > @@ -494,13 +496,6 @@ enum d71_blk_type {
> > >  #define D71_DEFAULT_PREPRETCH_LINE	5
> > >  #define D71_BUS_WIDTH_16_BYTES		16
> > >  
> > > -#define D71_MIN_LINE_SIZE		64
> > > -#define D71_MIN_VERTICAL_SIZE		64
> > > -#define D71_SC_MIN_LIN_SIZE		4
> > > -#define D71_SC_MIN_VERTICAL_SIZE	4
> > > -#define D71_SC_MAX_LIN_SIZE		2048
> > > -#define D71_SC_MAX_VERTICAL_SIZE	4096
> > > -
> > >  #define D71_SC_MAX_UPSCALING		64
> > >  #define D71_SC_MAX_DOWNSCALING		6
> > >  #define D71_SC_SPLIT_OVERLAP		8
> > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > index 910d279ae48d..92aba58ce2a5 100644
> > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > @@ -227,6 +227,8 @@ struct komeda_layer {
> > >  	/* accepted h/v input range before rotation */
> > >  	struct malidp_range hsize_in, vsize_in;
> > >  	u32 layer_type; /* RICH, SIMPLE or WB */
> > > +	u32 line_sz;
> > > +	u32 yuv_line_sz; /* maximum line size for YUV422 and YUV420 */
> > >  	u32 supported_rots;
> > >  	/* komeda supports layer split which splits a whole image to two parts
> > >  	 * left and right and handle them by two individual layer processors
> > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > index 5526731f5a33..6df442666cfe 100644
> > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > @@ -285,6 +285,7 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> > >  		       struct komeda_data_flow_cfg *dflow)
> > >  {
> > >  	u32 src_x, src_y, src_w, src_h;
> > > +	u32 line_sz, max_line_sz;
> > >  
> > >  	if (!komeda_fb_is_layer_supported(kfb, layer->layer_type, dflow->rot))
> > >  		return -EINVAL;
> > > @@ -314,6 +315,22 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> > >  		return -EINVAL;
> > >  	}
> > >  
> > > +	if (drm_rotation_90_or_270(dflow->rot))
> > > +		line_sz = dflow->in_h;
> > > +	else
> > > +		line_sz = dflow->in_w;
> > > +
> > > +	if (kfb->base.format->hsub > 1)
> > > +		max_line_sz = layer->yuv_line_sz;
> > > +	else
> > > +		max_line_sz = layer->line_sz;
> > > +
> > > +	if (line_sz > max_line_sz) {
> > > +		DRM_DEBUG_ATOMIC("Required line_sz: %d exceeds the max size %d\n",
> > > +				 line_sz, max_line_sz);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > >  	return 0;
> > >  }
> > >  
> > > -- 
> > > 2.17.1
> > > 
> > 
> > -- 
> > ====================
> > | I would like to |
> > | fix the world,  |
> > | but they're not |
> > | giving me the   |
> >  \ source code!  /
> >   ---------------
> >     ¯\_(ツ)_/¯

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v2 1/2] drm/komeda: Add line size support

[james qian wang (Arm Technology China)]

On Thu, Sep 26, 2019 at 11:51:21AM +0000, Liviu Dudau wrote:
> On Thu, Sep 26, 2019 at 10:00:22AM +0000, Lowry Li (Arm Technology China) wrote:
> > Hi Lowry,
> > On Wed, Sep 25, 2019 at 10:24:58AM +0000, Liviu Dudau wrote:
> > > Hi Lowry,
> > > 
> > > On Tue, Sep 24, 2019 at 08:00:44AM +0000, Lowry Li (Arm Technology China) wrote:
> > > > From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> > > > 
> > > > On D71, we are using the global line size. From D32, every
> > > > component have a line size register to indicate the fifo size.
> > > > 
> > > > So this patch is to set line size support and do the line size
> > > > check.
> > > > 
> > > > Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> > > > ---
> > > >  .../arm/display/komeda/d71/d71_component.c    | 57 ++++++++++++++++---
> > > >  .../gpu/drm/arm/display/komeda/d71/d71_regs.h |  9 +--
> > > >  .../drm/arm/display/komeda/komeda_pipeline.h  |  2 +
> > > >  .../display/komeda/komeda_pipeline_state.c    | 17 ++++++
> > > >  4 files changed, 70 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > index 7b374a3b911e..357837b9d6ed 100644
> > > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > @@ -106,6 +106,23 @@ static void dump_block_header(struct seq_file *sf, void __iomem *reg)
> > > >  			   i, hdr.output_ids[i]);
> > > >  }
> > > >  
> > > > +/* On D71, we are using the global line size. From D32, every component have
> > > > + * a line size register to indicate the fifo size.
> > > > + */
> > > > +static u32 __get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg,
> > > > +			       u32 max_default)
> > > > +{
> > > > +	if (!d71->periph_addr)
> > > > +		max_default = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > > +
> > > > +	return max_default;
> > > > +}
> > > > +
> > > > +static u32 get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg)
> > > > +{
> > > > +	return __get_blk_line_size(d71, reg, d71->max_line_size);
> > > > +}
> > > 
> > > I know you're trying to save typing the extra parameter, but looking at the rest of
> > > the diff I think it would look better if you get rid of get_blk_line_size() function
> > > and use the name for the function with 3 parameters.
> > > 
> > > Otherwise, patch looks good to me.
> > > 
> > > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> > > 
> > > Best regards,
> > > Liviu
> > Thanks for the comments.
> > But considering from D32 every component have a line size register
> > and no need default value, so we have get_blk_line_size() without
> > default argument and also can save some typing and lines. That's
> > why we want to keep __get_blk_line_size().
> 
> I was suggesting to remove get_blk_line_size and only use __get_blk_line_size() with
> explicit use of d71->max_line_size where it makes sense.
>

Hi Liviu:

Thank you for the suggestion.

Seems lowry doesn't describe it clearly.

the stroy is like this, for current komeda products:

- D71: Doesn't have per component line_size register to indicate the
       fifo size.
       And the fifo size is quite customized for every component,
       Alought for HW it is just a const value. but since it doesn't exposed
       to SW. So for driver It's quite annoy&hard to judage via lots of hints. 

- D32 or newer: have line_size register.

So for compatible with these two class products, we defined two functions:

- __get_blk_line_size():

  for d71 specific component like spliter/merger, that's why here we
  need input a default line_size, since all d71 specific component
  doesn't have its own line_size register or can not be indicated via
  the register GCU->line_size.
  Need to set it manually according to the specific component and lots
  of hints.

- get_blk_line_size(): 

  Two cases:
  -- d32 or newer: which have its own fifo line_size register
  -- d71: the line_size always same as the GCU->line_size
          register (the d71->max_line_size) 

So last as a conclusion:

- get_blk_line_size():
  is for the component that line_size can be indicated by HW line_size register,
  no matter component->line_size or GCU->line_size.

- __get_blk_line_size():
  for the d71 specific componet that needs manually calculate the line_size.

Thanks
James

> Best regards,
> Liviu
> 
> > 
> > > > +
> > > >  static u32 to_rot_ctrl(u32 rot)
> > > >  {
> > > >  	u32 lr_ctrl = 0;
> > > > @@ -365,7 +382,28 @@ static int d71_layer_init(struct d71_dev *d71,
> > > >  	else
> > > >  		layer->layer_type = KOMEDA_FMT_SIMPLE_LAYER;
> > > >  
> > > > -	set_range(&layer->hsize_in, 4, d71->max_line_size);
> > > > +	if (!d71->periph_addr) {
> > > > +		/* D32 or newer product */
> > > > +		layer->line_sz = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > > +		layer->yuv_line_sz = L_INFO_YUV_MAX_LINESZ(layer_info);
> > > > +	} else if (d71->max_line_size > 2048) {
> > > > +		/* D71 4K */
> > > > +		layer->line_sz = d71->max_line_size;
> > > > +		layer->yuv_line_sz = layer->line_sz / 2;
> > > > +	} else	{
> > > > +		/* D71 2K */
> > > > +		if (layer->layer_type == KOMEDA_FMT_RICH_LAYER) {
> > > > +			/* rich layer is 4K configuration */
> > > > +			layer->line_sz = d71->max_line_size * 2;
> > > > +			layer->yuv_line_sz = layer->line_sz / 2;
> > > > +		} else {
> > > > +			layer->line_sz = d71->max_line_size;
> > > > +			layer->yuv_line_sz = 0;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	set_range(&layer->hsize_in, 4, layer->line_sz);
> > > > +
> > > >  	set_range(&layer->vsize_in, 4, d71->max_vsize);
> > > >  
> > > >  	malidp_write32(reg, LAYER_PALPHA, D71_PALPHA_DEF_MAP);
> > > > @@ -456,9 +494,11 @@ static int d71_wb_layer_init(struct d71_dev *d71,
> > > >  
> > > >  	wb_layer = to_layer(c);
> > > >  	wb_layer->layer_type = KOMEDA_FMT_WB_LAYER;
> > > > +	wb_layer->line_sz = get_blk_line_size(d71, reg);
> > > > +	wb_layer->yuv_line_sz = wb_layer->line_sz;
> > > >  
> > > > -	set_range(&wb_layer->hsize_in, D71_MIN_LINE_SIZE, d71->max_line_size);
> > > > -	set_range(&wb_layer->vsize_in, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > > > +	set_range(&wb_layer->hsize_in, 64, wb_layer->line_sz);
> > > > +	set_range(&wb_layer->vsize_in, 64, d71->max_vsize);
> > > >  
> > > >  	return 0;
> > > >  }
> > > > @@ -595,8 +635,8 @@ static int d71_compiz_init(struct d71_dev *d71,
> > > >  
> > > >  	compiz = to_compiz(c);
> > > >  
> > > > -	set_range(&compiz->hsize, D71_MIN_LINE_SIZE, d71->max_line_size);
> > > > -	set_range(&compiz->vsize, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > > > +	set_range(&compiz->hsize, 64, get_blk_line_size(d71, reg));
> > > > +	set_range(&compiz->vsize, 64, d71->max_vsize);
> > > >  
> > > >  	return 0;
> > > >  }
> > > > @@ -753,7 +793,7 @@ static int d71_scaler_init(struct d71_dev *d71,
> > > >  	}
> > > >  
> > > >  	scaler = to_scaler(c);
> > > > -	set_range(&scaler->hsize, 4, 2048);
> > > > +	set_range(&scaler->hsize, 4, __get_blk_line_size(d71, reg, 2048));
> > > >  	set_range(&scaler->vsize, 4, 4096);
> > > >  	scaler->max_downscaling = 6;
> > > >  	scaler->max_upscaling = 64;
> > > > @@ -862,7 +902,7 @@ static int d71_splitter_init(struct d71_dev *d71,
> > > >  
> > > >  	splitter = to_splitter(c);
> > > >  
> > > > -	set_range(&splitter->hsize, 4, d71->max_line_size);
> > > > +	set_range(&splitter->hsize, 4, get_blk_line_size(d71, reg));
> > > >  	set_range(&splitter->vsize, 4, d71->max_vsize);
> > > >  
> > > >  	return 0;
> > > > @@ -933,7 +973,8 @@ static int d71_merger_init(struct d71_dev *d71,
> > > >  
> > > >  	merger = to_merger(c);
> > > >  
> > > > -	set_range(&merger->hsize_merged, 4, 4032);
> > > > +	set_range(&merger->hsize_merged, 4,
> > > > +		  __get_blk_line_size(d71, reg, 4032));
> > > >  	set_range(&merger->vsize_merged, 4, 4096);
> > > >  
> > > >  	return 0;
> > > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > index 2d5e6d00b42c..1727dc993909 100644
> > > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > @@ -10,6 +10,7 @@
> > > >  /* Common block registers offset */
> > > >  #define BLK_BLOCK_INFO		0x000
> > > >  #define BLK_PIPELINE_INFO	0x004
> > > > +#define BLK_MAX_LINE_SIZE	0x008
> > > >  #define BLK_VALID_INPUT_ID0	0x020
> > > >  #define BLK_OUTPUT_ID0		0x060
> > > >  #define BLK_INPUT_ID0		0x080
> > > > @@ -321,6 +322,7 @@
> > > >  #define L_INFO_RF		BIT(0)
> > > >  #define L_INFO_CM		BIT(1)
> > > >  #define L_INFO_ABUF_SIZE(x)	(((x) >> 4) & 0x7)
> > > > +#define L_INFO_YUV_MAX_LINESZ(x)	(((x) >> 16) & 0xFFFF)
> > > >  
> > > >  /* Scaler registers */
> > > >  #define SC_COEFFTAB		0x0DC
> > > > @@ -494,13 +496,6 @@ enum d71_blk_type {
> > > >  #define D71_DEFAULT_PREPRETCH_LINE	5
> > > >  #define D71_BUS_WIDTH_16_BYTES		16
> > > >  
> > > > -#define D71_MIN_LINE_SIZE		64
> > > > -#define D71_MIN_VERTICAL_SIZE		64
> > > > -#define D71_SC_MIN_LIN_SIZE		4
> > > > -#define D71_SC_MIN_VERTICAL_SIZE	4
> > > > -#define D71_SC_MAX_LIN_SIZE		2048
> > > > -#define D71_SC_MAX_VERTICAL_SIZE	4096
> > > > -
> > > >  #define D71_SC_MAX_UPSCALING		64
> > > >  #define D71_SC_MAX_DOWNSCALING		6
> > > >  #define D71_SC_SPLIT_OVERLAP		8
> > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > index 910d279ae48d..92aba58ce2a5 100644
> > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > @@ -227,6 +227,8 @@ struct komeda_layer {
> > > >  	/* accepted h/v input range before rotation */
> > > >  	struct malidp_range hsize_in, vsize_in;
> > > >  	u32 layer_type; /* RICH, SIMPLE or WB */
> > > > +	u32 line_sz;
> > > > +	u32 yuv_line_sz; /* maximum line size for YUV422 and YUV420 */
> > > >  	u32 supported_rots;
> > > >  	/* komeda supports layer split which splits a whole image to two parts
> > > >  	 * left and right and handle them by two individual layer processors
> > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > index 5526731f5a33..6df442666cfe 100644
> > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > @@ -285,6 +285,7 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> > > >  		       struct komeda_data_flow_cfg *dflow)
> > > >  {
> > > >  	u32 src_x, src_y, src_w, src_h;
> > > > +	u32 line_sz, max_line_sz;
> > > >  
> > > >  	if (!komeda_fb_is_layer_supported(kfb, layer->layer_type, dflow->rot))
> > > >  		return -EINVAL;
> > > > @@ -314,6 +315,22 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> > > >  		return -EINVAL;
> > > >  	}
> > > >  
> > > > +	if (drm_rotation_90_or_270(dflow->rot))
> > > > +		line_sz = dflow->in_h;
> > > > +	else
> > > > +		line_sz = dflow->in_w;
> > > > +
> > > > +	if (kfb->base.format->hsub > 1)
> > > > +		max_line_sz = layer->yuv_line_sz;
> > > > +	else
> > > > +		max_line_sz = layer->line_sz;
> > > > +
> > > > +	if (line_sz > max_line_sz) {
> > > > +		DRM_DEBUG_ATOMIC("Required line_sz: %d exceeds the max size %d\n",
> > > > +				 line_sz, max_line_sz);
> > > > +		return -EINVAL;
> > > > +	}
> > > > +
> > > >  	return 0;
> > > >  }
> > > >  
> > > > -- 
> > > > 2.17.1
> > > > 
> > > 
> > > -- 
> > > ====================
> > > | I would like to |
> > > | fix the world,  |
> > > | but they're not |
> > > | giving me the   |
> > >  \ source code!  /
> > >   ---------------
> > >     ¯\_(ツ)_/¯
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

Re: [PATCH v2 1/2] drm/komeda: Add line size support

[james qian wang (Arm Technology China)]

On Thu, Sep 26, 2019 at 11:51:21AM +0000, Liviu Dudau wrote:
> On Thu, Sep 26, 2019 at 10:00:22AM +0000, Lowry Li (Arm Technology China) wrote:
> > Hi Lowry,
> > On Wed, Sep 25, 2019 at 10:24:58AM +0000, Liviu Dudau wrote:
> > > Hi Lowry,
> > > 
> > > On Tue, Sep 24, 2019 at 08:00:44AM +0000, Lowry Li (Arm Technology China) wrote:
> > > > From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> > > > 
> > > > On D71, we are using the global line size. From D32, every
> > > > component have a line size register to indicate the fifo size.
> > > > 
> > > > So this patch is to set line size support and do the line size
> > > > check.
> > > > 
> > > > Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> > > > ---
> > > >  .../arm/display/komeda/d71/d71_component.c    | 57 ++++++++++++++++---
> > > >  .../gpu/drm/arm/display/komeda/d71/d71_regs.h |  9 +--
> > > >  .../drm/arm/display/komeda/komeda_pipeline.h  |  2 +
> > > >  .../display/komeda/komeda_pipeline_state.c    | 17 ++++++
> > > >  4 files changed, 70 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > index 7b374a3b911e..357837b9d6ed 100644
> > > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > @@ -106,6 +106,23 @@ static void dump_block_header(struct seq_file *sf, void __iomem *reg)
> > > >  			   i, hdr.output_ids[i]);
> > > >  }
> > > >  
> > > > +/* On D71, we are using the global line size. From D32, every component have
> > > > + * a line size register to indicate the fifo size.
> > > > + */
> > > > +static u32 __get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg,
> > > > +			       u32 max_default)
> > > > +{
> > > > +	if (!d71->periph_addr)
> > > > +		max_default = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > > +
> > > > +	return max_default;
> > > > +}
> > > > +
> > > > +static u32 get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg)
> > > > +{
> > > > +	return __get_blk_line_size(d71, reg, d71->max_line_size);
> > > > +}
> > > 
> > > I know you're trying to save typing the extra parameter, but looking at the rest of
> > > the diff I think it would look better if you get rid of get_blk_line_size() function
> > > and use the name for the function with 3 parameters.
> > > 
> > > Otherwise, patch looks good to me.
> > > 
> > > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> > > 
> > > Best regards,
> > > Liviu
> > Thanks for the comments.
> > But considering from D32 every component have a line size register
> > and no need default value, so we have get_blk_line_size() without
> > default argument and also can save some typing and lines. That's
> > why we want to keep __get_blk_line_size().
> 
> I was suggesting to remove get_blk_line_size and only use __get_blk_line_size() with
> explicit use of d71->max_line_size where it makes sense.
>

Hi Liviu:

Thank you for the suggestion.

Seems lowry doesn't describe it clearly.

the stroy is like this, for current komeda products:

- D71: Doesn't have per component line_size register to indicate the
       fifo size.
       And the fifo size is quite customized for every component,
       Alought for HW it is just a const value. but since it doesn't exposed
       to SW. So for driver It's quite annoy&hard to judage via lots of hints. 

- D32 or newer: have line_size register.

So for compatible with these two class products, we defined two functions:

- __get_blk_line_size():

  for d71 specific component like spliter/merger, that's why here we
  need input a default line_size, since all d71 specific component
  doesn't have its own line_size register or can not be indicated via
  the register GCU->line_size.
  Need to set it manually according to the specific component and lots
  of hints.

- get_blk_line_size(): 

  Two cases:
  -- d32 or newer: which have its own fifo line_size register
  -- d71: the line_size always same as the GCU->line_size
          register (the d71->max_line_size) 

So last as a conclusion:

- get_blk_line_size():
  is for the component that line_size can be indicated by HW line_size register,
  no matter component->line_size or GCU->line_size.

- __get_blk_line_size():
  for the d71 specific componet that needs manually calculate the line_size.

Thanks
James

> Best regards,
> Liviu
> 
> > 
> > > > +
> > > >  static u32 to_rot_ctrl(u32 rot)
> > > >  {
> > > >  	u32 lr_ctrl = 0;
> > > > @@ -365,7 +382,28 @@ static int d71_layer_init(struct d71_dev *d71,
> > > >  	else
> > > >  		layer->layer_type = KOMEDA_FMT_SIMPLE_LAYER;
> > > >  
> > > > -	set_range(&layer->hsize_in, 4, d71->max_line_size);
> > > > +	if (!d71->periph_addr) {
> > > > +		/* D32 or newer product */
> > > > +		layer->line_sz = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > > +		layer->yuv_line_sz = L_INFO_YUV_MAX_LINESZ(layer_info);
> > > > +	} else if (d71->max_line_size > 2048) {
> > > > +		/* D71 4K */
> > > > +		layer->line_sz = d71->max_line_size;
> > > > +		layer->yuv_line_sz = layer->line_sz / 2;
> > > > +	} else	{
> > > > +		/* D71 2K */
> > > > +		if (layer->layer_type == KOMEDA_FMT_RICH_LAYER) {
> > > > +			/* rich layer is 4K configuration */
> > > > +			layer->line_sz = d71->max_line_size * 2;
> > > > +			layer->yuv_line_sz = layer->line_sz / 2;
> > > > +		} else {
> > > > +			layer->line_sz = d71->max_line_size;
> > > > +			layer->yuv_line_sz = 0;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	set_range(&layer->hsize_in, 4, layer->line_sz);
> > > > +
> > > >  	set_range(&layer->vsize_in, 4, d71->max_vsize);
> > > >  
> > > >  	malidp_write32(reg, LAYER_PALPHA, D71_PALPHA_DEF_MAP);
> > > > @@ -456,9 +494,11 @@ static int d71_wb_layer_init(struct d71_dev *d71,
> > > >  
> > > >  	wb_layer = to_layer(c);
> > > >  	wb_layer->layer_type = KOMEDA_FMT_WB_LAYER;
> > > > +	wb_layer->line_sz = get_blk_line_size(d71, reg);
> > > > +	wb_layer->yuv_line_sz = wb_layer->line_sz;
> > > >  
> > > > -	set_range(&wb_layer->hsize_in, D71_MIN_LINE_SIZE, d71->max_line_size);
> > > > -	set_range(&wb_layer->vsize_in, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > > > +	set_range(&wb_layer->hsize_in, 64, wb_layer->line_sz);
> > > > +	set_range(&wb_layer->vsize_in, 64, d71->max_vsize);
> > > >  
> > > >  	return 0;
> > > >  }
> > > > @@ -595,8 +635,8 @@ static int d71_compiz_init(struct d71_dev *d71,
> > > >  
> > > >  	compiz = to_compiz(c);
> > > >  
> > > > -	set_range(&compiz->hsize, D71_MIN_LINE_SIZE, d71->max_line_size);
> > > > -	set_range(&compiz->vsize, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > > > +	set_range(&compiz->hsize, 64, get_blk_line_size(d71, reg));
> > > > +	set_range(&compiz->vsize, 64, d71->max_vsize);
> > > >  
> > > >  	return 0;
> > > >  }
> > > > @@ -753,7 +793,7 @@ static int d71_scaler_init(struct d71_dev *d71,
> > > >  	}
> > > >  
> > > >  	scaler = to_scaler(c);
> > > > -	set_range(&scaler->hsize, 4, 2048);
> > > > +	set_range(&scaler->hsize, 4, __get_blk_line_size(d71, reg, 2048));
> > > >  	set_range(&scaler->vsize, 4, 4096);
> > > >  	scaler->max_downscaling = 6;
> > > >  	scaler->max_upscaling = 64;
> > > > @@ -862,7 +902,7 @@ static int d71_splitter_init(struct d71_dev *d71,
> > > >  
> > > >  	splitter = to_splitter(c);
> > > >  
> > > > -	set_range(&splitter->hsize, 4, d71->max_line_size);
> > > > +	set_range(&splitter->hsize, 4, get_blk_line_size(d71, reg));
> > > >  	set_range(&splitter->vsize, 4, d71->max_vsize);
> > > >  
> > > >  	return 0;
> > > > @@ -933,7 +973,8 @@ static int d71_merger_init(struct d71_dev *d71,
> > > >  
> > > >  	merger = to_merger(c);
> > > >  
> > > > -	set_range(&merger->hsize_merged, 4, 4032);
> > > > +	set_range(&merger->hsize_merged, 4,
> > > > +		  __get_blk_line_size(d71, reg, 4032));
> > > >  	set_range(&merger->vsize_merged, 4, 4096);
> > > >  
> > > >  	return 0;
> > > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > index 2d5e6d00b42c..1727dc993909 100644
> > > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > @@ -10,6 +10,7 @@
> > > >  /* Common block registers offset */
> > > >  #define BLK_BLOCK_INFO		0x000
> > > >  #define BLK_PIPELINE_INFO	0x004
> > > > +#define BLK_MAX_LINE_SIZE	0x008
> > > >  #define BLK_VALID_INPUT_ID0	0x020
> > > >  #define BLK_OUTPUT_ID0		0x060
> > > >  #define BLK_INPUT_ID0		0x080
> > > > @@ -321,6 +322,7 @@
> > > >  #define L_INFO_RF		BIT(0)
> > > >  #define L_INFO_CM		BIT(1)
> > > >  #define L_INFO_ABUF_SIZE(x)	(((x) >> 4) & 0x7)
> > > > +#define L_INFO_YUV_MAX_LINESZ(x)	(((x) >> 16) & 0xFFFF)
> > > >  
> > > >  /* Scaler registers */
> > > >  #define SC_COEFFTAB		0x0DC
> > > > @@ -494,13 +496,6 @@ enum d71_blk_type {
> > > >  #define D71_DEFAULT_PREPRETCH_LINE	5
> > > >  #define D71_BUS_WIDTH_16_BYTES		16
> > > >  
> > > > -#define D71_MIN_LINE_SIZE		64
> > > > -#define D71_MIN_VERTICAL_SIZE		64
> > > > -#define D71_SC_MIN_LIN_SIZE		4
> > > > -#define D71_SC_MIN_VERTICAL_SIZE	4
> > > > -#define D71_SC_MAX_LIN_SIZE		2048
> > > > -#define D71_SC_MAX_VERTICAL_SIZE	4096
> > > > -
> > > >  #define D71_SC_MAX_UPSCALING		64
> > > >  #define D71_SC_MAX_DOWNSCALING		6
> > > >  #define D71_SC_SPLIT_OVERLAP		8
> > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > index 910d279ae48d..92aba58ce2a5 100644
> > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > @@ -227,6 +227,8 @@ struct komeda_layer {
> > > >  	/* accepted h/v input range before rotation */
> > > >  	struct malidp_range hsize_in, vsize_in;
> > > >  	u32 layer_type; /* RICH, SIMPLE or WB */
> > > > +	u32 line_sz;
> > > > +	u32 yuv_line_sz; /* maximum line size for YUV422 and YUV420 */
> > > >  	u32 supported_rots;
> > > >  	/* komeda supports layer split which splits a whole image to two parts
> > > >  	 * left and right and handle them by two individual layer processors
> > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > index 5526731f5a33..6df442666cfe 100644
> > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > @@ -285,6 +285,7 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> > > >  		       struct komeda_data_flow_cfg *dflow)
> > > >  {
> > > >  	u32 src_x, src_y, src_w, src_h;
> > > > +	u32 line_sz, max_line_sz;
> > > >  
> > > >  	if (!komeda_fb_is_layer_supported(kfb, layer->layer_type, dflow->rot))
> > > >  		return -EINVAL;
> > > > @@ -314,6 +315,22 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> > > >  		return -EINVAL;
> > > >  	}
> > > >  
> > > > +	if (drm_rotation_90_or_270(dflow->rot))
> > > > +		line_sz = dflow->in_h;
> > > > +	else
> > > > +		line_sz = dflow->in_w;
> > > > +
> > > > +	if (kfb->base.format->hsub > 1)
> > > > +		max_line_sz = layer->yuv_line_sz;
> > > > +	else
> > > > +		max_line_sz = layer->line_sz;
> > > > +
> > > > +	if (line_sz > max_line_sz) {
> > > > +		DRM_DEBUG_ATOMIC("Required line_sz: %d exceeds the max size %d\n",
> > > > +				 line_sz, max_line_sz);
> > > > +		return -EINVAL;
> > > > +	}
> > > > +
> > > >  	return 0;
> > > >  }
> > > >  
> > > > -- 
> > > > 2.17.1
> > > > 
> > > 
> > > -- 
> > > ====================
> > > | I would like to |
> > > | fix the world,  |
> > > | but they're not |
> > > | giving me the   |
> > >  \ source code!  /
> > >   ---------------
> > >     ¯\_(ツ)_/¯
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/2] drm/komeda: Add line size support

[Liviu Dudau]

On Fri, Sep 27, 2019 at 02:02:59AM +0000, james qian wang (Arm Technology China) wrote:
> On Thu, Sep 26, 2019 at 11:51:21AM +0000, Liviu Dudau wrote:
> > On Thu, Sep 26, 2019 at 10:00:22AM +0000, Lowry Li (Arm Technology China) wrote:
> > > Hi Lowry,
> > > On Wed, Sep 25, 2019 at 10:24:58AM +0000, Liviu Dudau wrote:
> > > > Hi Lowry,
> > > > 
> > > > On Tue, Sep 24, 2019 at 08:00:44AM +0000, Lowry Li (Arm Technology China) wrote:
> > > > > From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> > > > > 
> > > > > On D71, we are using the global line size. From D32, every
> > > > > component have a line size register to indicate the fifo size.
> > > > > 
> > > > > So this patch is to set line size support and do the line size
> > > > > check.
> > > > > 
> > > > > Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> > > > > ---
> > > > >  .../arm/display/komeda/d71/d71_component.c    | 57 ++++++++++++++++---
> > > > >  .../gpu/drm/arm/display/komeda/d71/d71_regs.h |  9 +--
> > > > >  .../drm/arm/display/komeda/komeda_pipeline.h  |  2 +
> > > > >  .../display/komeda/komeda_pipeline_state.c    | 17 ++++++
> > > > >  4 files changed, 70 insertions(+), 15 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > > index 7b374a3b911e..357837b9d6ed 100644
> > > > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > > @@ -106,6 +106,23 @@ static void dump_block_header(struct seq_file *sf, void __iomem *reg)
> > > > >  			   i, hdr.output_ids[i]);
> > > > >  }
> > > > >  
> > > > > +/* On D71, we are using the global line size. From D32, every component have
> > > > > + * a line size register to indicate the fifo size.
> > > > > + */
> > > > > +static u32 __get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg,
> > > > > +			       u32 max_default)
> > > > > +{
> > > > > +	if (!d71->periph_addr)
> > > > > +		max_default = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > > > +
> > > > > +	return max_default;
> > > > > +}
> > > > > +
> > > > > +static u32 get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg)
> > > > > +{
> > > > > +	return __get_blk_line_size(d71, reg, d71->max_line_size);
> > > > > +}
> > > > 
> > > > I know you're trying to save typing the extra parameter, but looking at the rest of
> > > > the diff I think it would look better if you get rid of get_blk_line_size() function
> > > > and use the name for the function with 3 parameters.
> > > > 
> > > > Otherwise, patch looks good to me.
> > > > 
> > > > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> > > > 
> > > > Best regards,
> > > > Liviu
> > > Thanks for the comments.
> > > But considering from D32 every component have a line size register
> > > and no need default value, so we have get_blk_line_size() without
> > > default argument and also can save some typing and lines. That's
> > > why we want to keep __get_blk_line_size().
> > 
> > I was suggesting to remove get_blk_line_size and only use __get_blk_line_size() with
> > explicit use of d71->max_line_size where it makes sense.
> >
> 
> Hi Liviu:

Hi James,

> 
> Thank you for the suggestion.
> 
> Seems lowry doesn't describe it clearly.
> 
> the stroy is like this, for current komeda products:
> 
> - D71: Doesn't have per component line_size register to indicate the
>        fifo size.
>        And the fifo size is quite customized for every component,
>        Alought for HW it is just a const value. but since it doesn't exposed
>        to SW. So for driver It's quite annoy&hard to judage via lots of hints. 
> 
> - D32 or newer: have line_size register.
> 
> So for compatible with these two class products, we defined two functions:
> 
> - __get_blk_line_size():
> 
>   for d71 specific component like spliter/merger, that's why here we
>   need input a default line_size, since all d71 specific component
>   doesn't have its own line_size register or can not be indicated via
>   the register GCU->line_size.
>   Need to set it manually according to the specific component and lots
>   of hints.
> 
> - get_blk_line_size(): 
> 
>   Two cases:
>   -- d32 or newer: which have its own fifo line_size register
>   -- d71: the line_size always same as the GCU->line_size
>           register (the d71->max_line_size) 
> 
> So last as a conclusion:
> 
> - get_blk_line_size():
>   is for the component that line_size can be indicated by HW line_size register,
>   no matter component->line_size or GCU->line_size.

Then call it get_hw_blk_line_size() :) But at the moment all it does is to call
__get_blk_line_size() with a well defined 3rd parameter.

> 
> - __get_blk_line_size():
>   for the d71 specific componet that needs manually calculate the line_size.

All I'm saying is that you could use __get_blk_line_size() everywhere and the diff
would look nicer because you could see that the same parameters are used.

We are also removing nice names that express what the values are with the actual
number, loosing some of the meta information that we had in the code.

From maintainability perspective I would say this patch does not help future
reviewers of the code.

Best regards,
Liviu

> 
> Thanks
> James
> 
> > Best regards,
> > Liviu
> > 
> > > 
> > > > > +
> > > > >  static u32 to_rot_ctrl(u32 rot)
> > > > >  {
> > > > >  	u32 lr_ctrl = 0;
> > > > > @@ -365,7 +382,28 @@ static int d71_layer_init(struct d71_dev *d71,
> > > > >  	else
> > > > >  		layer->layer_type = KOMEDA_FMT_SIMPLE_LAYER;
> > > > >  
> > > > > -	set_range(&layer->hsize_in, 4, d71->max_line_size);
> > > > > +	if (!d71->periph_addr) {
> > > > > +		/* D32 or newer product */
> > > > > +		layer->line_sz = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > > > +		layer->yuv_line_sz = L_INFO_YUV_MAX_LINESZ(layer_info);
> > > > > +	} else if (d71->max_line_size > 2048) {
> > > > > +		/* D71 4K */
> > > > > +		layer->line_sz = d71->max_line_size;
> > > > > +		layer->yuv_line_sz = layer->line_sz / 2;
> > > > > +	} else	{
> > > > > +		/* D71 2K */
> > > > > +		if (layer->layer_type == KOMEDA_FMT_RICH_LAYER) {
> > > > > +			/* rich layer is 4K configuration */
> > > > > +			layer->line_sz = d71->max_line_size * 2;
> > > > > +			layer->yuv_line_sz = layer->line_sz / 2;
> > > > > +		} else {
> > > > > +			layer->line_sz = d71->max_line_size;
> > > > > +			layer->yuv_line_sz = 0;
> > > > > +		}
> > > > > +	}
> > > > > +
> > > > > +	set_range(&layer->hsize_in, 4, layer->line_sz);
> > > > > +
> > > > >  	set_range(&layer->vsize_in, 4, d71->max_vsize);
> > > > >  
> > > > >  	malidp_write32(reg, LAYER_PALPHA, D71_PALPHA_DEF_MAP);
> > > > > @@ -456,9 +494,11 @@ static int d71_wb_layer_init(struct d71_dev *d71,
> > > > >  
> > > > >  	wb_layer = to_layer(c);
> > > > >  	wb_layer->layer_type = KOMEDA_FMT_WB_LAYER;
> > > > > +	wb_layer->line_sz = get_blk_line_size(d71, reg);
> > > > > +	wb_layer->yuv_line_sz = wb_layer->line_sz;
> > > > >  
> > > > > -	set_range(&wb_layer->hsize_in, D71_MIN_LINE_SIZE, d71->max_line_size);
> > > > > -	set_range(&wb_layer->vsize_in, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > > > > +	set_range(&wb_layer->hsize_in, 64, wb_layer->line_sz);
> > > > > +	set_range(&wb_layer->vsize_in, 64, d71->max_vsize);
> > > > >  
> > > > >  	return 0;
> > > > >  }
> > > > > @@ -595,8 +635,8 @@ static int d71_compiz_init(struct d71_dev *d71,
> > > > >  
> > > > >  	compiz = to_compiz(c);
> > > > >  
> > > > > -	set_range(&compiz->hsize, D71_MIN_LINE_SIZE, d71->max_line_size);
> > > > > -	set_range(&compiz->vsize, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > > > > +	set_range(&compiz->hsize, 64, get_blk_line_size(d71, reg));
> > > > > +	set_range(&compiz->vsize, 64, d71->max_vsize);
> > > > >  
> > > > >  	return 0;
> > > > >  }
> > > > > @@ -753,7 +793,7 @@ static int d71_scaler_init(struct d71_dev *d71,
> > > > >  	}
> > > > >  
> > > > >  	scaler = to_scaler(c);
> > > > > -	set_range(&scaler->hsize, 4, 2048);
> > > > > +	set_range(&scaler->hsize, 4, __get_blk_line_size(d71, reg, 2048));
> > > > >  	set_range(&scaler->vsize, 4, 4096);
> > > > >  	scaler->max_downscaling = 6;
> > > > >  	scaler->max_upscaling = 64;
> > > > > @@ -862,7 +902,7 @@ static int d71_splitter_init(struct d71_dev *d71,
> > > > >  
> > > > >  	splitter = to_splitter(c);
> > > > >  
> > > > > -	set_range(&splitter->hsize, 4, d71->max_line_size);
> > > > > +	set_range(&splitter->hsize, 4, get_blk_line_size(d71, reg));
> > > > >  	set_range(&splitter->vsize, 4, d71->max_vsize);
> > > > >  
> > > > >  	return 0;
> > > > > @@ -933,7 +973,8 @@ static int d71_merger_init(struct d71_dev *d71,
> > > > >  
> > > > >  	merger = to_merger(c);
> > > > >  
> > > > > -	set_range(&merger->hsize_merged, 4, 4032);
> > > > > +	set_range(&merger->hsize_merged, 4,
> > > > > +		  __get_blk_line_size(d71, reg, 4032));
> > > > >  	set_range(&merger->vsize_merged, 4, 4096);
> > > > >  
> > > > >  	return 0;
> > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > > index 2d5e6d00b42c..1727dc993909 100644
> > > > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > > @@ -10,6 +10,7 @@
> > > > >  /* Common block registers offset */
> > > > >  #define BLK_BLOCK_INFO		0x000
> > > > >  #define BLK_PIPELINE_INFO	0x004
> > > > > +#define BLK_MAX_LINE_SIZE	0x008
> > > > >  #define BLK_VALID_INPUT_ID0	0x020
> > > > >  #define BLK_OUTPUT_ID0		0x060
> > > > >  #define BLK_INPUT_ID0		0x080
> > > > > @@ -321,6 +322,7 @@
> > > > >  #define L_INFO_RF		BIT(0)
> > > > >  #define L_INFO_CM		BIT(1)
> > > > >  #define L_INFO_ABUF_SIZE(x)	(((x) >> 4) & 0x7)
> > > > > +#define L_INFO_YUV_MAX_LINESZ(x)	(((x) >> 16) & 0xFFFF)
> > > > >  
> > > > >  /* Scaler registers */
> > > > >  #define SC_COEFFTAB		0x0DC
> > > > > @@ -494,13 +496,6 @@ enum d71_blk_type {
> > > > >  #define D71_DEFAULT_PREPRETCH_LINE	5
> > > > >  #define D71_BUS_WIDTH_16_BYTES		16
> > > > >  
> > > > > -#define D71_MIN_LINE_SIZE		64
> > > > > -#define D71_MIN_VERTICAL_SIZE		64
> > > > > -#define D71_SC_MIN_LIN_SIZE		4
> > > > > -#define D71_SC_MIN_VERTICAL_SIZE	4
> > > > > -#define D71_SC_MAX_LIN_SIZE		2048
> > > > > -#define D71_SC_MAX_VERTICAL_SIZE	4096
> > > > > -
> > > > >  #define D71_SC_MAX_UPSCALING		64
> > > > >  #define D71_SC_MAX_DOWNSCALING		6
> > > > >  #define D71_SC_SPLIT_OVERLAP		8
> > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > > index 910d279ae48d..92aba58ce2a5 100644
> > > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > > @@ -227,6 +227,8 @@ struct komeda_layer {
> > > > >  	/* accepted h/v input range before rotation */
> > > > >  	struct malidp_range hsize_in, vsize_in;
> > > > >  	u32 layer_type; /* RICH, SIMPLE or WB */
> > > > > +	u32 line_sz;
> > > > > +	u32 yuv_line_sz; /* maximum line size for YUV422 and YUV420 */
> > > > >  	u32 supported_rots;
> > > > >  	/* komeda supports layer split which splits a whole image to two parts
> > > > >  	 * left and right and handle them by two individual layer processors
> > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > > index 5526731f5a33..6df442666cfe 100644
> > > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > > @@ -285,6 +285,7 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> > > > >  		       struct komeda_data_flow_cfg *dflow)
> > > > >  {
> > > > >  	u32 src_x, src_y, src_w, src_h;
> > > > > +	u32 line_sz, max_line_sz;
> > > > >  
> > > > >  	if (!komeda_fb_is_layer_supported(kfb, layer->layer_type, dflow->rot))
> > > > >  		return -EINVAL;
> > > > > @@ -314,6 +315,22 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> > > > >  		return -EINVAL;
> > > > >  	}
> > > > >  
> > > > > +	if (drm_rotation_90_or_270(dflow->rot))
> > > > > +		line_sz = dflow->in_h;
> > > > > +	else
> > > > > +		line_sz = dflow->in_w;
> > > > > +
> > > > > +	if (kfb->base.format->hsub > 1)
> > > > > +		max_line_sz = layer->yuv_line_sz;
> > > > > +	else
> > > > > +		max_line_sz = layer->line_sz;
> > > > > +
> > > > > +	if (line_sz > max_line_sz) {
> > > > > +		DRM_DEBUG_ATOMIC("Required line_sz: %d exceeds the max size %d\n",
> > > > > +				 line_sz, max_line_sz);
> > > > > +		return -EINVAL;
> > > > > +	}
> > > > > +
> > > > >  	return 0;
> > > > >  }
> > > > >  
> > > > > -- 
> > > > > 2.17.1
> > > > > 
> > > > 
> > > > -- 
> > > > ====================
> > > > | I would like to |
> > > > | fix the world,  |
> > > > | but they're not |
> > > > | giving me the   |
> > > >  \ source code!  /
> > > >   ---------------
> > > >     ¯\_(ツ)_/¯
> > 
> > -- 
> > ====================
> > | I would like to |
> > | fix the world,  |
> > | but they're not |
> > | giving me the   |
> >  \ source code!  /
> >   ---------------
> >     ¯\_(ツ)_/¯

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v2 1/2] drm/komeda: Add line size support

[Liviu Dudau]

On Fri, Sep 27, 2019 at 02:02:59AM +0000, james qian wang (Arm Technology China) wrote:
> On Thu, Sep 26, 2019 at 11:51:21AM +0000, Liviu Dudau wrote:
> > On Thu, Sep 26, 2019 at 10:00:22AM +0000, Lowry Li (Arm Technology China) wrote:
> > > Hi Lowry,
> > > On Wed, Sep 25, 2019 at 10:24:58AM +0000, Liviu Dudau wrote:
> > > > Hi Lowry,
> > > > 
> > > > On Tue, Sep 24, 2019 at 08:00:44AM +0000, Lowry Li (Arm Technology China) wrote:
> > > > > From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> > > > > 
> > > > > On D71, we are using the global line size. From D32, every
> > > > > component have a line size register to indicate the fifo size.
> > > > > 
> > > > > So this patch is to set line size support and do the line size
> > > > > check.
> > > > > 
> > > > > Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> > > > > ---
> > > > >  .../arm/display/komeda/d71/d71_component.c    | 57 ++++++++++++++++---
> > > > >  .../gpu/drm/arm/display/komeda/d71/d71_regs.h |  9 +--
> > > > >  .../drm/arm/display/komeda/komeda_pipeline.h  |  2 +
> > > > >  .../display/komeda/komeda_pipeline_state.c    | 17 ++++++
> > > > >  4 files changed, 70 insertions(+), 15 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > > index 7b374a3b911e..357837b9d6ed 100644
> > > > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > > @@ -106,6 +106,23 @@ static void dump_block_header(struct seq_file *sf, void __iomem *reg)
> > > > >  			   i, hdr.output_ids[i]);
> > > > >  }
> > > > >  
> > > > > +/* On D71, we are using the global line size. From D32, every component have
> > > > > + * a line size register to indicate the fifo size.
> > > > > + */
> > > > > +static u32 __get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg,
> > > > > +			       u32 max_default)
> > > > > +{
> > > > > +	if (!d71->periph_addr)
> > > > > +		max_default = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > > > +
> > > > > +	return max_default;
> > > > > +}
> > > > > +
> > > > > +static u32 get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg)
> > > > > +{
> > > > > +	return __get_blk_line_size(d71, reg, d71->max_line_size);
> > > > > +}
> > > > 
> > > > I know you're trying to save typing the extra parameter, but looking at the rest of
> > > > the diff I think it would look better if you get rid of get_blk_line_size() function
> > > > and use the name for the function with 3 parameters.
> > > > 
> > > > Otherwise, patch looks good to me.
> > > > 
> > > > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> > > > 
> > > > Best regards,
> > > > Liviu
> > > Thanks for the comments.
> > > But considering from D32 every component have a line size register
> > > and no need default value, so we have get_blk_line_size() without
> > > default argument and also can save some typing and lines. That's
> > > why we want to keep __get_blk_line_size().
> > 
> > I was suggesting to remove get_blk_line_size and only use __get_blk_line_size() with
> > explicit use of d71->max_line_size where it makes sense.
> >
> 
> Hi Liviu:

Hi James,

> 
> Thank you for the suggestion.
> 
> Seems lowry doesn't describe it clearly.
> 
> the stroy is like this, for current komeda products:
> 
> - D71: Doesn't have per component line_size register to indicate the
>        fifo size.
>        And the fifo size is quite customized for every component,
>        Alought for HW it is just a const value. but since it doesn't exposed
>        to SW. So for driver It's quite annoy&hard to judage via lots of hints. 
> 
> - D32 or newer: have line_size register.
> 
> So for compatible with these two class products, we defined two functions:
> 
> - __get_blk_line_size():
> 
>   for d71 specific component like spliter/merger, that's why here we
>   need input a default line_size, since all d71 specific component
>   doesn't have its own line_size register or can not be indicated via
>   the register GCU->line_size.
>   Need to set it manually according to the specific component and lots
>   of hints.
> 
> - get_blk_line_size(): 
> 
>   Two cases:
>   -- d32 or newer: which have its own fifo line_size register
>   -- d71: the line_size always same as the GCU->line_size
>           register (the d71->max_line_size) 
> 
> So last as a conclusion:
> 
> - get_blk_line_size():
>   is for the component that line_size can be indicated by HW line_size register,
>   no matter component->line_size or GCU->line_size.

Then call it get_hw_blk_line_size() :) But at the moment all it does is to call
__get_blk_line_size() with a well defined 3rd parameter.

> 
> - __get_blk_line_size():
>   for the d71 specific componet that needs manually calculate the line_size.

All I'm saying is that you could use __get_blk_line_size() everywhere and the diff
would look nicer because you could see that the same parameters are used.

We are also removing nice names that express what the values are with the actual
number, loosing some of the meta information that we had in the code.

>From maintainability perspective I would say this patch does not help future
reviewers of the code.

Best regards,
Liviu

> 
> Thanks
> James
> 
> > Best regards,
> > Liviu
> > 
> > > 
> > > > > +
> > > > >  static u32 to_rot_ctrl(u32 rot)
> > > > >  {
> > > > >  	u32 lr_ctrl = 0;
> > > > > @@ -365,7 +382,28 @@ static int d71_layer_init(struct d71_dev *d71,
> > > > >  	else
> > > > >  		layer->layer_type = KOMEDA_FMT_SIMPLE_LAYER;
> > > > >  
> > > > > -	set_range(&layer->hsize_in, 4, d71->max_line_size);
> > > > > +	if (!d71->periph_addr) {
> > > > > +		/* D32 or newer product */
> > > > > +		layer->line_sz = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > > > +		layer->yuv_line_sz = L_INFO_YUV_MAX_LINESZ(layer_info);
> > > > > +	} else if (d71->max_line_size > 2048) {
> > > > > +		/* D71 4K */
> > > > > +		layer->line_sz = d71->max_line_size;
> > > > > +		layer->yuv_line_sz = layer->line_sz / 2;
> > > > > +	} else	{
> > > > > +		/* D71 2K */
> > > > > +		if (layer->layer_type == KOMEDA_FMT_RICH_LAYER) {
> > > > > +			/* rich layer is 4K configuration */
> > > > > +			layer->line_sz = d71->max_line_size * 2;
> > > > > +			layer->yuv_line_sz = layer->line_sz / 2;
> > > > > +		} else {
> > > > > +			layer->line_sz = d71->max_line_size;
> > > > > +			layer->yuv_line_sz = 0;
> > > > > +		}
> > > > > +	}
> > > > > +
> > > > > +	set_range(&layer->hsize_in, 4, layer->line_sz);
> > > > > +
> > > > >  	set_range(&layer->vsize_in, 4, d71->max_vsize);
> > > > >  
> > > > >  	malidp_write32(reg, LAYER_PALPHA, D71_PALPHA_DEF_MAP);
> > > > > @@ -456,9 +494,11 @@ static int d71_wb_layer_init(struct d71_dev *d71,
> > > > >  
> > > > >  	wb_layer = to_layer(c);
> > > > >  	wb_layer->layer_type = KOMEDA_FMT_WB_LAYER;
> > > > > +	wb_layer->line_sz = get_blk_line_size(d71, reg);
> > > > > +	wb_layer->yuv_line_sz = wb_layer->line_sz;
> > > > >  
> > > > > -	set_range(&wb_layer->hsize_in, D71_MIN_LINE_SIZE, d71->max_line_size);
> > > > > -	set_range(&wb_layer->vsize_in, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > > > > +	set_range(&wb_layer->hsize_in, 64, wb_layer->line_sz);
> > > > > +	set_range(&wb_layer->vsize_in, 64, d71->max_vsize);
> > > > >  
> > > > >  	return 0;
> > > > >  }
> > > > > @@ -595,8 +635,8 @@ static int d71_compiz_init(struct d71_dev *d71,
> > > > >  
> > > > >  	compiz = to_compiz(c);
> > > > >  
> > > > > -	set_range(&compiz->hsize, D71_MIN_LINE_SIZE, d71->max_line_size);
> > > > > -	set_range(&compiz->vsize, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > > > > +	set_range(&compiz->hsize, 64, get_blk_line_size(d71, reg));
> > > > > +	set_range(&compiz->vsize, 64, d71->max_vsize);
> > > > >  
> > > > >  	return 0;
> > > > >  }
> > > > > @@ -753,7 +793,7 @@ static int d71_scaler_init(struct d71_dev *d71,
> > > > >  	}
> > > > >  
> > > > >  	scaler = to_scaler(c);
> > > > > -	set_range(&scaler->hsize, 4, 2048);
> > > > > +	set_range(&scaler->hsize, 4, __get_blk_line_size(d71, reg, 2048));
> > > > >  	set_range(&scaler->vsize, 4, 4096);
> > > > >  	scaler->max_downscaling = 6;
> > > > >  	scaler->max_upscaling = 64;
> > > > > @@ -862,7 +902,7 @@ static int d71_splitter_init(struct d71_dev *d71,
> > > > >  
> > > > >  	splitter = to_splitter(c);
> > > > >  
> > > > > -	set_range(&splitter->hsize, 4, d71->max_line_size);
> > > > > +	set_range(&splitter->hsize, 4, get_blk_line_size(d71, reg));
> > > > >  	set_range(&splitter->vsize, 4, d71->max_vsize);
> > > > >  
> > > > >  	return 0;
> > > > > @@ -933,7 +973,8 @@ static int d71_merger_init(struct d71_dev *d71,
> > > > >  
> > > > >  	merger = to_merger(c);
> > > > >  
> > > > > -	set_range(&merger->hsize_merged, 4, 4032);
> > > > > +	set_range(&merger->hsize_merged, 4,
> > > > > +		  __get_blk_line_size(d71, reg, 4032));
> > > > >  	set_range(&merger->vsize_merged, 4, 4096);
> > > > >  
> > > > >  	return 0;
> > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > > index 2d5e6d00b42c..1727dc993909 100644
> > > > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > > @@ -10,6 +10,7 @@
> > > > >  /* Common block registers offset */
> > > > >  #define BLK_BLOCK_INFO		0x000
> > > > >  #define BLK_PIPELINE_INFO	0x004
> > > > > +#define BLK_MAX_LINE_SIZE	0x008
> > > > >  #define BLK_VALID_INPUT_ID0	0x020
> > > > >  #define BLK_OUTPUT_ID0		0x060
> > > > >  #define BLK_INPUT_ID0		0x080
> > > > > @@ -321,6 +322,7 @@
> > > > >  #define L_INFO_RF		BIT(0)
> > > > >  #define L_INFO_CM		BIT(1)
> > > > >  #define L_INFO_ABUF_SIZE(x)	(((x) >> 4) & 0x7)
> > > > > +#define L_INFO_YUV_MAX_LINESZ(x)	(((x) >> 16) & 0xFFFF)
> > > > >  
> > > > >  /* Scaler registers */
> > > > >  #define SC_COEFFTAB		0x0DC
> > > > > @@ -494,13 +496,6 @@ enum d71_blk_type {
> > > > >  #define D71_DEFAULT_PREPRETCH_LINE	5
> > > > >  #define D71_BUS_WIDTH_16_BYTES		16
> > > > >  
> > > > > -#define D71_MIN_LINE_SIZE		64
> > > > > -#define D71_MIN_VERTICAL_SIZE		64
> > > > > -#define D71_SC_MIN_LIN_SIZE		4
> > > > > -#define D71_SC_MIN_VERTICAL_SIZE	4
> > > > > -#define D71_SC_MAX_LIN_SIZE		2048
> > > > > -#define D71_SC_MAX_VERTICAL_SIZE	4096
> > > > > -
> > > > >  #define D71_SC_MAX_UPSCALING		64
> > > > >  #define D71_SC_MAX_DOWNSCALING		6
> > > > >  #define D71_SC_SPLIT_OVERLAP		8
> > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > > index 910d279ae48d..92aba58ce2a5 100644
> > > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > > @@ -227,6 +227,8 @@ struct komeda_layer {
> > > > >  	/* accepted h/v input range before rotation */
> > > > >  	struct malidp_range hsize_in, vsize_in;
> > > > >  	u32 layer_type; /* RICH, SIMPLE or WB */
> > > > > +	u32 line_sz;
> > > > > +	u32 yuv_line_sz; /* maximum line size for YUV422 and YUV420 */
> > > > >  	u32 supported_rots;
> > > > >  	/* komeda supports layer split which splits a whole image to two parts
> > > > >  	 * left and right and handle them by two individual layer processors
> > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > > index 5526731f5a33..6df442666cfe 100644
> > > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > > @@ -285,6 +285,7 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> > > > >  		       struct komeda_data_flow_cfg *dflow)
> > > > >  {
> > > > >  	u32 src_x, src_y, src_w, src_h;
> > > > > +	u32 line_sz, max_line_sz;
> > > > >  
> > > > >  	if (!komeda_fb_is_layer_supported(kfb, layer->layer_type, dflow->rot))
> > > > >  		return -EINVAL;
> > > > > @@ -314,6 +315,22 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> > > > >  		return -EINVAL;
> > > > >  	}
> > > > >  
> > > > > +	if (drm_rotation_90_or_270(dflow->rot))
> > > > > +		line_sz = dflow->in_h;
> > > > > +	else
> > > > > +		line_sz = dflow->in_w;
> > > > > +
> > > > > +	if (kfb->base.format->hsub > 1)
> > > > > +		max_line_sz = layer->yuv_line_sz;
> > > > > +	else
> > > > > +		max_line_sz = layer->line_sz;
> > > > > +
> > > > > +	if (line_sz > max_line_sz) {
> > > > > +		DRM_DEBUG_ATOMIC("Required line_sz: %d exceeds the max size %d\n",
> > > > > +				 line_sz, max_line_sz);
> > > > > +		return -EINVAL;
> > > > > +	}
> > > > > +
> > > > >  	return 0;
> > > > >  }
> > > > >  
> > > > > -- 
> > > > > 2.17.1
> > > > > 
> > > > 
> > > > -- 
> > > > ====================
> > > > | I would like to |
> > > > | fix the world,  |
> > > > | but they're not |
> > > > | giving me the   |
> > > >  \ source code!  /
> > > >   ---------------
> > > >     ¯\_(ツ)_/¯
> > 
> > -- 
> > ====================
> > | I would like to |
> > | fix the world,  |
> > | but they're not |
> > | giving me the   |
> >  \ source code!  /
> >   ---------------
> >     ¯\_(ツ)_/¯

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v2 1/2] drm/komeda: Add line size support

[james qian wang (Arm Technology China)]

On Fri, Sep 27, 2019 at 12:42:13PM +0000, Liviu Dudau wrote:
> On Fri, Sep 27, 2019 at 02:02:59AM +0000, james qian wang (Arm Technology China) wrote:
> > On Thu, Sep 26, 2019 at 11:51:21AM +0000, Liviu Dudau wrote:
> > > On Thu, Sep 26, 2019 at 10:00:22AM +0000, Lowry Li (Arm Technology China) wrote:
> > > > Hi Lowry,
> > > > On Wed, Sep 25, 2019 at 10:24:58AM +0000, Liviu Dudau wrote:
> > > > > Hi Lowry,
> > > > > 
> > > > > On Tue, Sep 24, 2019 at 08:00:44AM +0000, Lowry Li (Arm Technology China) wrote:
> > > > > > From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> > > > > > 
> > > > > > On D71, we are using the global line size. From D32, every
> > > > > > component have a line size register to indicate the fifo size.
> > > > > > 
> > > > > > So this patch is to set line size support and do the line size
> > > > > > check.
> > > > > > 
> > > > > > Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> > > > > > ---
> > > > > >  .../arm/display/komeda/d71/d71_component.c    | 57 ++++++++++++++++---
> > > > > >  .../gpu/drm/arm/display/komeda/d71/d71_regs.h |  9 +--
> > > > > >  .../drm/arm/display/komeda/komeda_pipeline.h  |  2 +
> > > > > >  .../display/komeda/komeda_pipeline_state.c    | 17 ++++++
> > > > > >  4 files changed, 70 insertions(+), 15 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > > > index 7b374a3b911e..357837b9d6ed 100644
> > > > > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > > > @@ -106,6 +106,23 @@ static void dump_block_header(struct seq_file *sf, void __iomem *reg)
> > > > > >  			   i, hdr.output_ids[i]);
> > > > > >  }
> > > > > >  
> > > > > > +/* On D71, we are using the global line size. From D32, every component have
> > > > > > + * a line size register to indicate the fifo size.
> > > > > > + */
> > > > > > +static u32 __get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg,
> > > > > > +			       u32 max_default)
> > > > > > +{
> > > > > > +	if (!d71->periph_addr)
> > > > > > +		max_default = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > > > > +
> > > > > > +	return max_default;
> > > > > > +}
> > > > > > +
> > > > > > +static u32 get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg)
> > > > > > +{
> > > > > > +	return __get_blk_line_size(d71, reg, d71->max_line_size);
> > > > > > +}
> > > > > 
> > > > > I know you're trying to save typing the extra parameter, but looking at the rest of
> > > > > the diff I think it would look better if you get rid of get_blk_line_size() function
> > > > > and use the name for the function with 3 parameters.
> > > > > 
> > > > > Otherwise, patch looks good to me.
> > > > > 
> > > > > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Reviewed-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
> > > > > 
> > > > > Best regards,
> > > > > Liviu
> > > > Thanks for the comments.
> > > > But considering from D32 every component have a line size register
> > > > and no need default value, so we have get_blk_line_size() without
> > > > default argument and also can save some typing and lines. That's
> > > > why we want to keep __get_blk_line_size().
> > > 
> > > I was suggesting to remove get_blk_line_size and only use __get_blk_line_size() with
> > > explicit use of d71->max_line_size where it makes sense.
> > >
> > 
> > Hi Liviu:
> 
> Hi James,
> 
> > 
> > Thank you for the suggestion.
> > 
> > Seems lowry doesn't describe it clearly.
> > 
> > the stroy is like this, for current komeda products:
> > 
> > - D71: Doesn't have per component line_size register to indicate the
> >        fifo size.
> >        And the fifo size is quite customized for every component,
> >        Alought for HW it is just a const value. but since it doesn't exposed
> >        to SW. So for driver It's quite annoy&hard to judage via lots of hints. 
> > 
> > - D32 or newer: have line_size register.
> > 
> > So for compatible with these two class products, we defined two functions:
> > 
> > - __get_blk_line_size():
> > 
> >   for d71 specific component like spliter/merger, that's why here we
> >   need input a default line_size, since all d71 specific component
> >   doesn't have its own line_size register or can not be indicated via
> >   the register GCU->line_size.
> >   Need to set it manually according to the specific component and lots
> >   of hints.
> > 
> > - get_blk_line_size(): 
> > 
> >   Two cases:
> >   -- d32 or newer: which have its own fifo line_size register
> >   -- d71: the line_size always same as the GCU->line_size
> >           register (the d71->max_line_size) 
> > 
> > So last as a conclusion:
> > 
> > - get_blk_line_size():
> >   is for the component that line_size can be indicated by HW line_size register,
> >   no matter component->line_size or GCU->line_size.
> 
> Then call it get_hw_blk_line_size() :) But at the moment all it does is to call
> __get_blk_line_size() with a well defined 3rd parameter.
> 
> > 
> > - __get_blk_line_size():
> >   for the d71 specific componet that needs manually calculate the line_size.
> 
> All I'm saying is that you could use __get_blk_line_size() everywhere and the diff
> would look nicer because you could see that the same parameters are used.
> 
> We are also removing nice names that express what the values are with the actual
> number, loosing some of the meta information that we had in the code.
> 
> From maintainability perspective I would say this patch does not help future
> reviewers of the code.
> 
> Best regards,
> Liviu
>

Hi Liviu:

Sorry, I have different opinion.

The maintainence, That's why we defined two funcs:

- __get_blk_line_size(..., u32 max_default)

  this is only for old d71 and some special components.

- get_blk_line_size(), directly get the line_size from HW.

  this is for the most d71 components, and all component in newer product,
  no need to care about the default values.

For most component, __get_blk_line_size(... u32 max_default) just imports complexity
and conflusion. but the func get_blk_line_size() is more preferabled.

Best Regards
James
> > 
> > Thanks
> > James
> > 
> > > Best regards,
> > > Liviu
> > > 
> > > > 
> > > > > > +
> > > > > >  static u32 to_rot_ctrl(u32 rot)
> > > > > >  {
> > > > > >  	u32 lr_ctrl = 0;
> > > > > > @@ -365,7 +382,28 @@ static int d71_layer_init(struct d71_dev *d71,
> > > > > >  	else
> > > > > >  		layer->layer_type = KOMEDA_FMT_SIMPLE_LAYER;
> > > > > >  
> > > > > > -	set_range(&layer->hsize_in, 4, d71->max_line_size);
> > > > > > +	if (!d71->periph_addr) {
> > > > > > +		/* D32 or newer product */
> > > > > > +		layer->line_sz = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > > > > +		layer->yuv_line_sz = L_INFO_YUV_MAX_LINESZ(layer_info);
> > > > > > +	} else if (d71->max_line_size > 2048) {
> > > > > > +		/* D71 4K */
> > > > > > +		layer->line_sz = d71->max_line_size;
> > > > > > +		layer->yuv_line_sz = layer->line_sz / 2;
> > > > > > +	} else	{
> > > > > > +		/* D71 2K */
> > > > > > +		if (layer->layer_type == KOMEDA_FMT_RICH_LAYER) {
> > > > > > +			/* rich layer is 4K configuration */
> > > > > > +			layer->line_sz = d71->max_line_size * 2;
> > > > > > +			layer->yuv_line_sz = layer->line_sz / 2;
> > > > > > +		} else {
> > > > > > +			layer->line_sz = d71->max_line_size;
> > > > > > +			layer->yuv_line_sz = 0;
> > > > > > +		}
> > > > > > +	}
> > > > > > +
> > > > > > +	set_range(&layer->hsize_in, 4, layer->line_sz);
> > > > > > +
> > > > > >  	set_range(&layer->vsize_in, 4, d71->max_vsize);
> > > > > >  
> > > > > >  	malidp_write32(reg, LAYER_PALPHA, D71_PALPHA_DEF_MAP);
> > > > > > @@ -456,9 +494,11 @@ static int d71_wb_layer_init(struct d71_dev *d71,
> > > > > >  
> > > > > >  	wb_layer = to_layer(c);
> > > > > >  	wb_layer->layer_type = KOMEDA_FMT_WB_LAYER;
> > > > > > +	wb_layer->line_sz = get_blk_line_size(d71, reg);
> > > > > > +	wb_layer->yuv_line_sz = wb_layer->line_sz;
> > > > > >  
> > > > > > -	set_range(&wb_layer->hsize_in, D71_MIN_LINE_SIZE, d71->max_line_size);
> > > > > > -	set_range(&wb_layer->vsize_in, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > > > > > +	set_range(&wb_layer->hsize_in, 64, wb_layer->line_sz);
> > > > > > +	set_range(&wb_layer->vsize_in, 64, d71->max_vsize);
> > > > > >  
> > > > > >  	return 0;
> > > > > >  }
> > > > > > @@ -595,8 +635,8 @@ static int d71_compiz_init(struct d71_dev *d71,
> > > > > >  
> > > > > >  	compiz = to_compiz(c);
> > > > > >  
> > > > > > -	set_range(&compiz->hsize, D71_MIN_LINE_SIZE, d71->max_line_size);
> > > > > > -	set_range(&compiz->vsize, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > > > > > +	set_range(&compiz->hsize, 64, get_blk_line_size(d71, reg));
> > > > > > +	set_range(&compiz->vsize, 64, d71->max_vsize);
> > > > > >  
> > > > > >  	return 0;
> > > > > >  }
> > > > > > @@ -753,7 +793,7 @@ static int d71_scaler_init(struct d71_dev *d71,
> > > > > >  	}
> > > > > >  
> > > > > >  	scaler = to_scaler(c);
> > > > > > -	set_range(&scaler->hsize, 4, 2048);
> > > > > > +	set_range(&scaler->hsize, 4, __get_blk_line_size(d71, reg, 2048));
> > > > > >  	set_range(&scaler->vsize, 4, 4096);
> > > > > >  	scaler->max_downscaling = 6;
> > > > > >  	scaler->max_upscaling = 64;
> > > > > > @@ -862,7 +902,7 @@ static int d71_splitter_init(struct d71_dev *d71,
> > > > > >  
> > > > > >  	splitter = to_splitter(c);
> > > > > >  
> > > > > > -	set_range(&splitter->hsize, 4, d71->max_line_size);
> > > > > > +	set_range(&splitter->hsize, 4, get_blk_line_size(d71, reg));
> > > > > >  	set_range(&splitter->vsize, 4, d71->max_vsize);
> > > > > >  
> > > > > >  	return 0;
> > > > > > @@ -933,7 +973,8 @@ static int d71_merger_init(struct d71_dev *d71,
> > > > > >  
> > > > > >  	merger = to_merger(c);
> > > > > >  
> > > > > > -	set_range(&merger->hsize_merged, 4, 4032);
> > > > > > +	set_range(&merger->hsize_merged, 4,
> > > > > > +		  __get_blk_line_size(d71, reg, 4032));
> > > > > >  	set_range(&merger->vsize_merged, 4, 4096);
> > > > > >  
> > > > > >  	return 0;
> > > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > > > index 2d5e6d00b42c..1727dc993909 100644
> > > > > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > > > @@ -10,6 +10,7 @@
> > > > > >  /* Common block registers offset */
> > > > > >  #define BLK_BLOCK_INFO		0x000
> > > > > >  #define BLK_PIPELINE_INFO	0x004
> > > > > > +#define BLK_MAX_LINE_SIZE	0x008
> > > > > >  #define BLK_VALID_INPUT_ID0	0x020
> > > > > >  #define BLK_OUTPUT_ID0		0x060
> > > > > >  #define BLK_INPUT_ID0		0x080
> > > > > > @@ -321,6 +322,7 @@
> > > > > >  #define L_INFO_RF		BIT(0)
> > > > > >  #define L_INFO_CM		BIT(1)
> > > > > >  #define L_INFO_ABUF_SIZE(x)	(((x) >> 4) & 0x7)
> > > > > > +#define L_INFO_YUV_MAX_LINESZ(x)	(((x) >> 16) & 0xFFFF)
> > > > > >  
> > > > > >  /* Scaler registers */
> > > > > >  #define SC_COEFFTAB		0x0DC
> > > > > > @@ -494,13 +496,6 @@ enum d71_blk_type {
> > > > > >  #define D71_DEFAULT_PREPRETCH_LINE	5
> > > > > >  #define D71_BUS_WIDTH_16_BYTES		16
> > > > > >  
> > > > > > -#define D71_MIN_LINE_SIZE		64
> > > > > > -#define D71_MIN_VERTICAL_SIZE		64
> > > > > > -#define D71_SC_MIN_LIN_SIZE		4
> > > > > > -#define D71_SC_MIN_VERTICAL_SIZE	4
> > > > > > -#define D71_SC_MAX_LIN_SIZE		2048
> > > > > > -#define D71_SC_MAX_VERTICAL_SIZE	4096
> > > > > > -
> > > > > >  #define D71_SC_MAX_UPSCALING		64
> > > > > >  #define D71_SC_MAX_DOWNSCALING		6
> > > > > >  #define D71_SC_SPLIT_OVERLAP		8
> > > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > > > index 910d279ae48d..92aba58ce2a5 100644
> > > > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > > > @@ -227,6 +227,8 @@ struct komeda_layer {
> > > > > >  	/* accepted h/v input range before rotation */
> > > > > >  	struct malidp_range hsize_in, vsize_in;
> > > > > >  	u32 layer_type; /* RICH, SIMPLE or WB */
> > > > > > +	u32 line_sz;
> > > > > > +	u32 yuv_line_sz; /* maximum line size for YUV422 and YUV420 */
> > > > > >  	u32 supported_rots;
> > > > > >  	/* komeda supports layer split which splits a whole image to two parts
> > > > > >  	 * left and right and handle them by two individual layer processors
> > > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > > > index 5526731f5a33..6df442666cfe 100644
> > > > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > > > @@ -285,6 +285,7 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> > > > > >  		       struct komeda_data_flow_cfg *dflow)
> > > > > >  {
> > > > > >  	u32 src_x, src_y, src_w, src_h;
> > > > > > +	u32 line_sz, max_line_sz;
> > > > > >  
> > > > > >  	if (!komeda_fb_is_layer_supported(kfb, layer->layer_type, dflow->rot))
> > > > > >  		return -EINVAL;
> > > > > > @@ -314,6 +315,22 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> > > > > >  		return -EINVAL;
> > > > > >  	}
> > > > > >  
> > > > > > +	if (drm_rotation_90_or_270(dflow->rot))
> > > > > > +		line_sz = dflow->in_h;
> > > > > > +	else
> > > > > > +		line_sz = dflow->in_w;
> > > > > > +
> > > > > > +	if (kfb->base.format->hsub > 1)
> > > > > > +		max_line_sz = layer->yuv_line_sz;
> > > > > > +	else
> > > > > > +		max_line_sz = layer->line_sz;
> > > > > > +
> > > > > > +	if (line_sz > max_line_sz) {
> > > > > > +		DRM_DEBUG_ATOMIC("Required line_sz: %d exceeds the max size %d\n",
> > > > > > +				 line_sz, max_line_sz);
> > > > > > +		return -EINVAL;
> > > > > > +	}
> > > > > > +
> > > > > >  	return 0;
> > > > > >  }
> > > > > >  
> > > > > > -- 
> > > > > > 2.17.1
> > > > > > 
> > > > > 
> > > > > -- 
> > > > > ====================
> > > > > | I would like to |
> > > > > | fix the world,  |
> > > > > | but they're not |
> > > > > | giving me the   |
> > > > >  \ source code!  /
> > > > >   ---------------
> > > > >     ¯\_(ツ)_/¯
> > > 
> > > -- 
> > > ====================
> > > | I would like to |
> > > | fix the world,  |
> > > | but they're not |
> > > | giving me the   |
> > >  \ source code!  /
> > >   ---------------
> > >     ¯\_(ツ)_/¯
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

Re: [PATCH v2 1/2] drm/komeda: Add line size support

[james qian wang (Arm Technology China)]

On Fri, Sep 27, 2019 at 12:42:13PM +0000, Liviu Dudau wrote:
> On Fri, Sep 27, 2019 at 02:02:59AM +0000, james qian wang (Arm Technology China) wrote:
> > On Thu, Sep 26, 2019 at 11:51:21AM +0000, Liviu Dudau wrote:
> > > On Thu, Sep 26, 2019 at 10:00:22AM +0000, Lowry Li (Arm Technology China) wrote:
> > > > Hi Lowry,
> > > > On Wed, Sep 25, 2019 at 10:24:58AM +0000, Liviu Dudau wrote:
> > > > > Hi Lowry,
> > > > > 
> > > > > On Tue, Sep 24, 2019 at 08:00:44AM +0000, Lowry Li (Arm Technology China) wrote:
> > > > > > From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> > > > > > 
> > > > > > On D71, we are using the global line size. From D32, every
> > > > > > component have a line size register to indicate the fifo size.
> > > > > > 
> > > > > > So this patch is to set line size support and do the line size
> > > > > > check.
> > > > > > 
> > > > > > Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> > > > > > ---
> > > > > >  .../arm/display/komeda/d71/d71_component.c    | 57 ++++++++++++++++---
> > > > > >  .../gpu/drm/arm/display/komeda/d71/d71_regs.h |  9 +--
> > > > > >  .../drm/arm/display/komeda/komeda_pipeline.h  |  2 +
> > > > > >  .../display/komeda/komeda_pipeline_state.c    | 17 ++++++
> > > > > >  4 files changed, 70 insertions(+), 15 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > > > index 7b374a3b911e..357837b9d6ed 100644
> > > > > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > > > @@ -106,6 +106,23 @@ static void dump_block_header(struct seq_file *sf, void __iomem *reg)
> > > > > >  			   i, hdr.output_ids[i]);
> > > > > >  }
> > > > > >  
> > > > > > +/* On D71, we are using the global line size. From D32, every component have
> > > > > > + * a line size register to indicate the fifo size.
> > > > > > + */
> > > > > > +static u32 __get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg,
> > > > > > +			       u32 max_default)
> > > > > > +{
> > > > > > +	if (!d71->periph_addr)
> > > > > > +		max_default = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > > > > +
> > > > > > +	return max_default;
> > > > > > +}
> > > > > > +
> > > > > > +static u32 get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg)
> > > > > > +{
> > > > > > +	return __get_blk_line_size(d71, reg, d71->max_line_size);
> > > > > > +}
> > > > > 
> > > > > I know you're trying to save typing the extra parameter, but looking at the rest of
> > > > > the diff I think it would look better if you get rid of get_blk_line_size() function
> > > > > and use the name for the function with 3 parameters.
> > > > > 
> > > > > Otherwise, patch looks good to me.
> > > > > 
> > > > > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Reviewed-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
> > > > > 
> > > > > Best regards,
> > > > > Liviu
> > > > Thanks for the comments.
> > > > But considering from D32 every component have a line size register
> > > > and no need default value, so we have get_blk_line_size() without
> > > > default argument and also can save some typing and lines. That's
> > > > why we want to keep __get_blk_line_size().
> > > 
> > > I was suggesting to remove get_blk_line_size and only use __get_blk_line_size() with
> > > explicit use of d71->max_line_size where it makes sense.
> > >
> > 
> > Hi Liviu:
> 
> Hi James,
> 
> > 
> > Thank you for the suggestion.
> > 
> > Seems lowry doesn't describe it clearly.
> > 
> > the stroy is like this, for current komeda products:
> > 
> > - D71: Doesn't have per component line_size register to indicate the
> >        fifo size.
> >        And the fifo size is quite customized for every component,
> >        Alought for HW it is just a const value. but since it doesn't exposed
> >        to SW. So for driver It's quite annoy&hard to judage via lots of hints. 
> > 
> > - D32 or newer: have line_size register.
> > 
> > So for compatible with these two class products, we defined two functions:
> > 
> > - __get_blk_line_size():
> > 
> >   for d71 specific component like spliter/merger, that's why here we
> >   need input a default line_size, since all d71 specific component
> >   doesn't have its own line_size register or can not be indicated via
> >   the register GCU->line_size.
> >   Need to set it manually according to the specific component and lots
> >   of hints.
> > 
> > - get_blk_line_size(): 
> > 
> >   Two cases:
> >   -- d32 or newer: which have its own fifo line_size register
> >   -- d71: the line_size always same as the GCU->line_size
> >           register (the d71->max_line_size) 
> > 
> > So last as a conclusion:
> > 
> > - get_blk_line_size():
> >   is for the component that line_size can be indicated by HW line_size register,
> >   no matter component->line_size or GCU->line_size.
> 
> Then call it get_hw_blk_line_size() :) But at the moment all it does is to call
> __get_blk_line_size() with a well defined 3rd parameter.
> 
> > 
> > - __get_blk_line_size():
> >   for the d71 specific componet that needs manually calculate the line_size.
> 
> All I'm saying is that you could use __get_blk_line_size() everywhere and the diff
> would look nicer because you could see that the same parameters are used.
> 
> We are also removing nice names that express what the values are with the actual
> number, loosing some of the meta information that we had in the code.
> 
> From maintainability perspective I would say this patch does not help future
> reviewers of the code.
> 
> Best regards,
> Liviu
>

Hi Liviu:

Sorry, I have different opinion.

The maintainence, That's why we defined two funcs:

- __get_blk_line_size(..., u32 max_default)

  this is only for old d71 and some special components.

- get_blk_line_size(), directly get the line_size from HW.

  this is for the most d71 components, and all component in newer product,
  no need to care about the default values.

For most component, __get_blk_line_size(... u32 max_default) just imports complexity
and conflusion. but the func get_blk_line_size() is more preferabled.

Best Regards
James
> > 
> > Thanks
> > James
> > 
> > > Best regards,
> > > Liviu
> > > 
> > > > 
> > > > > > +
> > > > > >  static u32 to_rot_ctrl(u32 rot)
> > > > > >  {
> > > > > >  	u32 lr_ctrl = 0;
> > > > > > @@ -365,7 +382,28 @@ static int d71_layer_init(struct d71_dev *d71,
> > > > > >  	else
> > > > > >  		layer->layer_type = KOMEDA_FMT_SIMPLE_LAYER;
> > > > > >  
> > > > > > -	set_range(&layer->hsize_in, 4, d71->max_line_size);
> > > > > > +	if (!d71->periph_addr) {
> > > > > > +		/* D32 or newer product */
> > > > > > +		layer->line_sz = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > > > > +		layer->yuv_line_sz = L_INFO_YUV_MAX_LINESZ(layer_info);
> > > > > > +	} else if (d71->max_line_size > 2048) {
> > > > > > +		/* D71 4K */
> > > > > > +		layer->line_sz = d71->max_line_size;
> > > > > > +		layer->yuv_line_sz = layer->line_sz / 2;
> > > > > > +	} else	{
> > > > > > +		/* D71 2K */
> > > > > > +		if (layer->layer_type == KOMEDA_FMT_RICH_LAYER) {
> > > > > > +			/* rich layer is 4K configuration */
> > > > > > +			layer->line_sz = d71->max_line_size * 2;
> > > > > > +			layer->yuv_line_sz = layer->line_sz / 2;
> > > > > > +		} else {
> > > > > > +			layer->line_sz = d71->max_line_size;
> > > > > > +			layer->yuv_line_sz = 0;
> > > > > > +		}
> > > > > > +	}
> > > > > > +
> > > > > > +	set_range(&layer->hsize_in, 4, layer->line_sz);
> > > > > > +
> > > > > >  	set_range(&layer->vsize_in, 4, d71->max_vsize);
> > > > > >  
> > > > > >  	malidp_write32(reg, LAYER_PALPHA, D71_PALPHA_DEF_MAP);
> > > > > > @@ -456,9 +494,11 @@ static int d71_wb_layer_init(struct d71_dev *d71,
> > > > > >  
> > > > > >  	wb_layer = to_layer(c);
> > > > > >  	wb_layer->layer_type = KOMEDA_FMT_WB_LAYER;
> > > > > > +	wb_layer->line_sz = get_blk_line_size(d71, reg);
> > > > > > +	wb_layer->yuv_line_sz = wb_layer->line_sz;
> > > > > >  
> > > > > > -	set_range(&wb_layer->hsize_in, D71_MIN_LINE_SIZE, d71->max_line_size);
> > > > > > -	set_range(&wb_layer->vsize_in, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > > > > > +	set_range(&wb_layer->hsize_in, 64, wb_layer->line_sz);
> > > > > > +	set_range(&wb_layer->vsize_in, 64, d71->max_vsize);
> > > > > >  
> > > > > >  	return 0;
> > > > > >  }
> > > > > > @@ -595,8 +635,8 @@ static int d71_compiz_init(struct d71_dev *d71,
> > > > > >  
> > > > > >  	compiz = to_compiz(c);
> > > > > >  
> > > > > > -	set_range(&compiz->hsize, D71_MIN_LINE_SIZE, d71->max_line_size);
> > > > > > -	set_range(&compiz->vsize, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > > > > > +	set_range(&compiz->hsize, 64, get_blk_line_size(d71, reg));
> > > > > > +	set_range(&compiz->vsize, 64, d71->max_vsize);
> > > > > >  
> > > > > >  	return 0;
> > > > > >  }
> > > > > > @@ -753,7 +793,7 @@ static int d71_scaler_init(struct d71_dev *d71,
> > > > > >  	}
> > > > > >  
> > > > > >  	scaler = to_scaler(c);
> > > > > > -	set_range(&scaler->hsize, 4, 2048);
> > > > > > +	set_range(&scaler->hsize, 4, __get_blk_line_size(d71, reg, 2048));
> > > > > >  	set_range(&scaler->vsize, 4, 4096);
> > > > > >  	scaler->max_downscaling = 6;
> > > > > >  	scaler->max_upscaling = 64;
> > > > > > @@ -862,7 +902,7 @@ static int d71_splitter_init(struct d71_dev *d71,
> > > > > >  
> > > > > >  	splitter = to_splitter(c);
> > > > > >  
> > > > > > -	set_range(&splitter->hsize, 4, d71->max_line_size);
> > > > > > +	set_range(&splitter->hsize, 4, get_blk_line_size(d71, reg));
> > > > > >  	set_range(&splitter->vsize, 4, d71->max_vsize);
> > > > > >  
> > > > > >  	return 0;
> > > > > > @@ -933,7 +973,8 @@ static int d71_merger_init(struct d71_dev *d71,
> > > > > >  
> > > > > >  	merger = to_merger(c);
> > > > > >  
> > > > > > -	set_range(&merger->hsize_merged, 4, 4032);
> > > > > > +	set_range(&merger->hsize_merged, 4,
> > > > > > +		  __get_blk_line_size(d71, reg, 4032));
> > > > > >  	set_range(&merger->vsize_merged, 4, 4096);
> > > > > >  
> > > > > >  	return 0;
> > > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > > > index 2d5e6d00b42c..1727dc993909 100644
> > > > > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_regs.h
> > > > > > @@ -10,6 +10,7 @@
> > > > > >  /* Common block registers offset */
> > > > > >  #define BLK_BLOCK_INFO		0x000
> > > > > >  #define BLK_PIPELINE_INFO	0x004
> > > > > > +#define BLK_MAX_LINE_SIZE	0x008
> > > > > >  #define BLK_VALID_INPUT_ID0	0x020
> > > > > >  #define BLK_OUTPUT_ID0		0x060
> > > > > >  #define BLK_INPUT_ID0		0x080
> > > > > > @@ -321,6 +322,7 @@
> > > > > >  #define L_INFO_RF		BIT(0)
> > > > > >  #define L_INFO_CM		BIT(1)
> > > > > >  #define L_INFO_ABUF_SIZE(x)	(((x) >> 4) & 0x7)
> > > > > > +#define L_INFO_YUV_MAX_LINESZ(x)	(((x) >> 16) & 0xFFFF)
> > > > > >  
> > > > > >  /* Scaler registers */
> > > > > >  #define SC_COEFFTAB		0x0DC
> > > > > > @@ -494,13 +496,6 @@ enum d71_blk_type {
> > > > > >  #define D71_DEFAULT_PREPRETCH_LINE	5
> > > > > >  #define D71_BUS_WIDTH_16_BYTES		16
> > > > > >  
> > > > > > -#define D71_MIN_LINE_SIZE		64
> > > > > > -#define D71_MIN_VERTICAL_SIZE		64
> > > > > > -#define D71_SC_MIN_LIN_SIZE		4
> > > > > > -#define D71_SC_MIN_VERTICAL_SIZE	4
> > > > > > -#define D71_SC_MAX_LIN_SIZE		2048
> > > > > > -#define D71_SC_MAX_VERTICAL_SIZE	4096
> > > > > > -
> > > > > >  #define D71_SC_MAX_UPSCALING		64
> > > > > >  #define D71_SC_MAX_DOWNSCALING		6
> > > > > >  #define D71_SC_SPLIT_OVERLAP		8
> > > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > > > index 910d279ae48d..92aba58ce2a5 100644
> > > > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > > > > > @@ -227,6 +227,8 @@ struct komeda_layer {
> > > > > >  	/* accepted h/v input range before rotation */
> > > > > >  	struct malidp_range hsize_in, vsize_in;
> > > > > >  	u32 layer_type; /* RICH, SIMPLE or WB */
> > > > > > +	u32 line_sz;
> > > > > > +	u32 yuv_line_sz; /* maximum line size for YUV422 and YUV420 */
> > > > > >  	u32 supported_rots;
> > > > > >  	/* komeda supports layer split which splits a whole image to two parts
> > > > > >  	 * left and right and handle them by two individual layer processors
> > > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > > > index 5526731f5a33..6df442666cfe 100644
> > > > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > > > @@ -285,6 +285,7 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> > > > > >  		       struct komeda_data_flow_cfg *dflow)
> > > > > >  {
> > > > > >  	u32 src_x, src_y, src_w, src_h;
> > > > > > +	u32 line_sz, max_line_sz;
> > > > > >  
> > > > > >  	if (!komeda_fb_is_layer_supported(kfb, layer->layer_type, dflow->rot))
> > > > > >  		return -EINVAL;
> > > > > > @@ -314,6 +315,22 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> > > > > >  		return -EINVAL;
> > > > > >  	}
> > > > > >  
> > > > > > +	if (drm_rotation_90_or_270(dflow->rot))
> > > > > > +		line_sz = dflow->in_h;
> > > > > > +	else
> > > > > > +		line_sz = dflow->in_w;
> > > > > > +
> > > > > > +	if (kfb->base.format->hsub > 1)
> > > > > > +		max_line_sz = layer->yuv_line_sz;
> > > > > > +	else
> > > > > > +		max_line_sz = layer->line_sz;
> > > > > > +
> > > > > > +	if (line_sz > max_line_sz) {
> > > > > > +		DRM_DEBUG_ATOMIC("Required line_sz: %d exceeds the max size %d\n",
> > > > > > +				 line_sz, max_line_sz);
> > > > > > +		return -EINVAL;
> > > > > > +	}
> > > > > > +
> > > > > >  	return 0;
> > > > > >  }
> > > > > >  
> > > > > > -- 
> > > > > > 2.17.1
> > > > > > 
> > > > > 
> > > > > -- 
> > > > > ====================
> > > > > | I would like to |
> > > > > | fix the world,  |
> > > > > | but they're not |
> > > > > | giving me the   |
> > > > >  \ source code!  /
> > > > >   ---------------
> > > > >     ¯\_(ツ)_/¯
> > > 
> > > -- 
> > > ====================
> > > | I would like to |
> > > | fix the world,  |
> > > | but they're not |
> > > | giving me the   |
> > >  \ source code!  /
> > >   ---------------
> > >     ¯\_(ツ)_/¯
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/2] drm/komeda: Adds layer horizontal input size limitation check for D71

[Lowry Li (Arm Technology China)]

From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>

Adds maximum line size check according to the AFBC decoder limitation
and special Line size limitation(2046) for format: YUV420_10BIT and X0L2.

Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
---
 .../arm/display/komeda/d71/d71_component.c    | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
index 357837b9d6ed..6740b8422f11 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
@@ -349,7 +349,56 @@ static void d71_layer_dump(struct komeda_component *c, struct seq_file *sf)
 	seq_printf(sf, "%sAD_V_CROP:\t\t0x%X\n", prefix, v[2]);
 }
 
+static int d71_layer_validate(struct komeda_component *c,
+			      struct komeda_component_state *state)
+{
+	struct komeda_layer_state *st = to_layer_st(state);
+	struct komeda_layer *layer = to_layer(c);
+	struct drm_plane_state *plane_st;
+	struct drm_framebuffer *fb;
+	u32 fourcc, line_sz, max_line_sz;
+
+	plane_st = drm_atomic_get_new_plane_state(state->obj.state,
+						  state->plane);
+	fb = plane_st->fb;
+	fourcc = fb->format->format;
+
+	if (drm_rotation_90_or_270(st->rot))
+		line_sz = st->vsize - st->afbc_crop_t - st->afbc_crop_b;
+	else
+		line_sz = st->hsize - st->afbc_crop_l - st->afbc_crop_r;
+
+	if (fb->modifier) {
+		if ((fb->modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) ==
+			AFBC_FORMAT_MOD_BLOCK_SIZE_32x8)
+			max_line_sz = layer->line_sz;
+		else
+			max_line_sz = layer->line_sz / 2;
+
+		if (line_sz > max_line_sz) {
+			DRM_DEBUG_ATOMIC("afbc request line_sz: %d exceed the max afbc line_sz: %d.\n",
+					 line_sz, max_line_sz);
+			return -EINVAL;
+		}
+	}
+
+	if (fourcc == DRM_FORMAT_YUV420_10BIT && line_sz > 2046 && (st->afbc_crop_l % 4)) {
+		DRM_DEBUG_ATOMIC("YUV420_10BIT input_hsize: %d exceed the max size 2046.\n",
+				 line_sz);
+		return -EINVAL;
+	}
+
+	if (fourcc == DRM_FORMAT_X0L2 && line_sz > 2046 && (st->addr[0] % 16)) {
+		DRM_DEBUG_ATOMIC("X0L2 input_hsize: %d exceed the max size 2046.\n",
+				 line_sz);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static const struct komeda_component_funcs d71_layer_funcs = {
+	.validate	= d71_layer_validate,
 	.update		= d71_layer_update,
 	.disable	= d71_layer_disable,
 	.dump_register	= d71_layer_dump,
-- 
2.17.1

[PATCH v2 2/2] drm/komeda: Adds layer horizontal input size limitation check for D71

[Lowry Li (Arm Technology China)]

From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>

Adds maximum line size check according to the AFBC decoder limitation
and special Line size limitation(2046) for format: YUV420_10BIT and X0L2.

Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
---
 .../arm/display/komeda/d71/d71_component.c    | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
index 357837b9d6ed..6740b8422f11 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
@@ -349,7 +349,56 @@ static void d71_layer_dump(struct komeda_component *c, struct seq_file *sf)
 	seq_printf(sf, "%sAD_V_CROP:\t\t0x%X\n", prefix, v[2]);
 }
 
+static int d71_layer_validate(struct komeda_component *c,
+			      struct komeda_component_state *state)
+{
+	struct komeda_layer_state *st = to_layer_st(state);
+	struct komeda_layer *layer = to_layer(c);
+	struct drm_plane_state *plane_st;
+	struct drm_framebuffer *fb;
+	u32 fourcc, line_sz, max_line_sz;
+
+	plane_st = drm_atomic_get_new_plane_state(state->obj.state,
+						  state->plane);
+	fb = plane_st->fb;
+	fourcc = fb->format->format;
+
+	if (drm_rotation_90_or_270(st->rot))
+		line_sz = st->vsize - st->afbc_crop_t - st->afbc_crop_b;
+	else
+		line_sz = st->hsize - st->afbc_crop_l - st->afbc_crop_r;
+
+	if (fb->modifier) {
+		if ((fb->modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) ==
+			AFBC_FORMAT_MOD_BLOCK_SIZE_32x8)
+			max_line_sz = layer->line_sz;
+		else
+			max_line_sz = layer->line_sz / 2;
+
+		if (line_sz > max_line_sz) {
+			DRM_DEBUG_ATOMIC("afbc request line_sz: %d exceed the max afbc line_sz: %d.\n",
+					 line_sz, max_line_sz);
+			return -EINVAL;
+		}
+	}
+
+	if (fourcc == DRM_FORMAT_YUV420_10BIT && line_sz > 2046 && (st->afbc_crop_l % 4)) {
+		DRM_DEBUG_ATOMIC("YUV420_10BIT input_hsize: %d exceed the max size 2046.\n",
+				 line_sz);
+		return -EINVAL;
+	}
+
+	if (fourcc == DRM_FORMAT_X0L2 && line_sz > 2046 && (st->addr[0] % 16)) {
+		DRM_DEBUG_ATOMIC("X0L2 input_hsize: %d exceed the max size 2046.\n",
+				 line_sz);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static const struct komeda_component_funcs d71_layer_funcs = {
+	.validate	= d71_layer_validate,
 	.update		= d71_layer_update,
 	.disable	= d71_layer_disable,
 	.dump_register	= d71_layer_dump,
-- 
2.17.1

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

Re: [PATCH v2 2/2] drm/komeda: Adds layer horizontal input size limitation check for D71

[Liviu Dudau]

On Tue, Sep 24, 2019 at 08:00:49AM +0000, Lowry Li (Arm Technology China) wrote:
> From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> 
> Adds maximum line size check according to the AFBC decoder limitation
> and special Line size limitation(2046) for format: YUV420_10BIT and X0L2.
> 
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu

> ---
>  .../arm/display/komeda/d71/d71_component.c    | 49 +++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> index 357837b9d6ed..6740b8422f11 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> @@ -349,7 +349,56 @@ static void d71_layer_dump(struct komeda_component *c, struct seq_file *sf)
>  	seq_printf(sf, "%sAD_V_CROP:\t\t0x%X\n", prefix, v[2]);
>  }
>  
> +static int d71_layer_validate(struct komeda_component *c,
> +			      struct komeda_component_state *state)
> +{
> +	struct komeda_layer_state *st = to_layer_st(state);
> +	struct komeda_layer *layer = to_layer(c);
> +	struct drm_plane_state *plane_st;
> +	struct drm_framebuffer *fb;
> +	u32 fourcc, line_sz, max_line_sz;
> +
> +	plane_st = drm_atomic_get_new_plane_state(state->obj.state,
> +						  state->plane);
> +	fb = plane_st->fb;
> +	fourcc = fb->format->format;
> +
> +	if (drm_rotation_90_or_270(st->rot))
> +		line_sz = st->vsize - st->afbc_crop_t - st->afbc_crop_b;
> +	else
> +		line_sz = st->hsize - st->afbc_crop_l - st->afbc_crop_r;
> +
> +	if (fb->modifier) {
> +		if ((fb->modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) ==
> +			AFBC_FORMAT_MOD_BLOCK_SIZE_32x8)
> +			max_line_sz = layer->line_sz;
> +		else
> +			max_line_sz = layer->line_sz / 2;
> +
> +		if (line_sz > max_line_sz) {
> +			DRM_DEBUG_ATOMIC("afbc request line_sz: %d exceed the max afbc line_sz: %d.\n",
> +					 line_sz, max_line_sz);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if (fourcc == DRM_FORMAT_YUV420_10BIT && line_sz > 2046 && (st->afbc_crop_l % 4)) {
> +		DRM_DEBUG_ATOMIC("YUV420_10BIT input_hsize: %d exceed the max size 2046.\n",
> +				 line_sz);
> +		return -EINVAL;
> +	}
> +
> +	if (fourcc == DRM_FORMAT_X0L2 && line_sz > 2046 && (st->addr[0] % 16)) {
> +		DRM_DEBUG_ATOMIC("X0L2 input_hsize: %d exceed the max size 2046.\n",
> +				 line_sz);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct komeda_component_funcs d71_layer_funcs = {
> +	.validate	= d71_layer_validate,
>  	.update		= d71_layer_update,
>  	.disable	= d71_layer_disable,
>  	.dump_register	= d71_layer_dump,
> -- 
> 2.17.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v2 2/2] drm/komeda: Adds layer horizontal input size limitation check for D71

[Liviu Dudau]

On Tue, Sep 24, 2019 at 08:00:49AM +0000, Lowry Li (Arm Technology China) wrote:
> From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> 
> Adds maximum line size check according to the AFBC decoder limitation
> and special Line size limitation(2046) for format: YUV420_10BIT and X0L2.
> 
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu

> ---
>  .../arm/display/komeda/d71/d71_component.c    | 49 +++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> index 357837b9d6ed..6740b8422f11 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> @@ -349,7 +349,56 @@ static void d71_layer_dump(struct komeda_component *c, struct seq_file *sf)
>  	seq_printf(sf, "%sAD_V_CROP:\t\t0x%X\n", prefix, v[2]);
>  }
>  
> +static int d71_layer_validate(struct komeda_component *c,
> +			      struct komeda_component_state *state)
> +{
> +	struct komeda_layer_state *st = to_layer_st(state);
> +	struct komeda_layer *layer = to_layer(c);
> +	struct drm_plane_state *plane_st;
> +	struct drm_framebuffer *fb;
> +	u32 fourcc, line_sz, max_line_sz;
> +
> +	plane_st = drm_atomic_get_new_plane_state(state->obj.state,
> +						  state->plane);
> +	fb = plane_st->fb;
> +	fourcc = fb->format->format;
> +
> +	if (drm_rotation_90_or_270(st->rot))
> +		line_sz = st->vsize - st->afbc_crop_t - st->afbc_crop_b;
> +	else
> +		line_sz = st->hsize - st->afbc_crop_l - st->afbc_crop_r;
> +
> +	if (fb->modifier) {
> +		if ((fb->modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) ==
> +			AFBC_FORMAT_MOD_BLOCK_SIZE_32x8)
> +			max_line_sz = layer->line_sz;
> +		else
> +			max_line_sz = layer->line_sz / 2;
> +
> +		if (line_sz > max_line_sz) {
> +			DRM_DEBUG_ATOMIC("afbc request line_sz: %d exceed the max afbc line_sz: %d.\n",
> +					 line_sz, max_line_sz);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if (fourcc == DRM_FORMAT_YUV420_10BIT && line_sz > 2046 && (st->afbc_crop_l % 4)) {
> +		DRM_DEBUG_ATOMIC("YUV420_10BIT input_hsize: %d exceed the max size 2046.\n",
> +				 line_sz);
> +		return -EINVAL;
> +	}
> +
> +	if (fourcc == DRM_FORMAT_X0L2 && line_sz > 2046 && (st->addr[0] % 16)) {
> +		DRM_DEBUG_ATOMIC("X0L2 input_hsize: %d exceed the max size 2046.\n",
> +				 line_sz);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct komeda_component_funcs d71_layer_funcs = {
> +	.validate	= d71_layer_validate,
>  	.update		= d71_layer_update,
>  	.disable	= d71_layer_disable,
>  	.dump_register	= d71_layer_dump,
> -- 
> 2.17.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

[PATCH v2 0/2] drm/komeda: Add layer line size support

[Lowry Li (Arm Technology China)]

Hi,

From D32 every component have a line size register to indicate internal
fifo size, instead of using the global line_sz.

This serie aims at adding the layer line size support and check
accordingly on both D71 and D32 or newer.

Depends on:
https://patchwork.freedesktop.org/series/62377/
https://patchwork.freedesktop.org/series/62181/

Changes since v1:
Rebases to drm-misc-next branch.

Lowry Li (Arm Technology China) (2):
  drm/komeda: Add line size support
  drm/komeda: Adds layer horizontal input size limitation check for D71

 .../arm/display/komeda/d71/d71_component.c    | 106 ++++++++++++++++--
 .../gpu/drm/arm/display/komeda/d71/d71_regs.h |   9 +-
 .../drm/arm/display/komeda/komeda_pipeline.h  |   2 +
 .../display/komeda/komeda_pipeline_state.c    |  17 +++
 4 files changed, 119 insertions(+), 15 deletions(-)

-- 
2.17.1

[PATCH v2 0/2] drm/komeda: Add layer line size support

[Lowry Li (Arm Technology China)]

Hi,

>From D32 every component have a line size register to indicate internal
fifo size, instead of using the global line_sz.

This serie aims at adding the layer line size support and check
accordingly on both D71 and D32 or newer.

Depends on:
https://patchwork.freedesktop.org/series/62377/
https://patchwork.freedesktop.org/series/62181/

Changes since v1:
Rebases to drm-misc-next branch.

Lowry Li (Arm Technology China) (2):
  drm/komeda: Add line size support
  drm/komeda: Adds layer horizontal input size limitation check for D71

 .../arm/display/komeda/d71/d71_component.c    | 106 ++++++++++++++++--
 .../gpu/drm/arm/display/komeda/d71/d71_regs.h |   9 +-
 .../drm/arm/display/komeda/komeda_pipeline.h  |   2 +
 .../display/komeda/komeda_pipeline_state.c    |  17 +++
 4 files changed, 119 insertions(+), 15 deletions(-)

-- 
2.17.1