diff mbox series

[v14,12/15] drm/mediatek: dpi: Add YUV422 output support

Message ID 20220624030946.14961-13-rex-bc.chen@mediatek.com (mailing list archive)
State New, archived
Headers show
Series drm/mediatek: Add MT8195 dp_intf driver | expand

Commit Message

Rex-BC Chen (陳柏辰) June 24, 2022, 3:09 a.m. UTC 
Dp_intf supports YUV422 as output format. In MT8195 Chrome project,
YUV422 output format is used for 4K resolution.

To support this, it is also needed to support color format transfer.
Color format transfer is a new feature for both dpi and dpintf of MT8195.

The input format could be RGB888 and output format for dp_intf should be
YUV422. Therefore, we add a mtk_dpi_matrix_sel() helper to update the
DPI_MATRIX_SET register depending on the color format.

Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
Signed-off-by: Bo-Chen Chen <rex-bc.chen@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_dpi.c      | 34 ++++++++++++++++++++++++-
 drivers/gpu/drm/mediatek/mtk_dpi_regs.h |  3 +++
 2 files changed, 36 insertions(+), 1 deletion(-)

Comments

CK Hu (胡俊光) June 28, 2022, 2:15 a.m. UTC | #1 
Hi, Bo-Chen:

On Fri, 2022-06-24 at 11:09 +0800, Bo-Chen Chen wrote:
> Dp_intf supports YUV422 as output format. In MT8195 Chrome project,
> YUV422 output format is used for 4K resolution.
> 
> To support this, it is also needed to support color format transfer.
> Color format transfer is a new feature for both dpi and dpintf of
> MT8195.
> 
> The input format could be RGB888 and output format for dp_intf should
> be
> YUV422. Therefore, we add a mtk_dpi_matrix_sel() helper to update the
> DPI_MATRIX_SET register depending on the color format.
> 
> Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
> Signed-off-by: Bo-Chen Chen <rex-bc.chen@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_dpi.c      | 34
> ++++++++++++++++++++++++-
>  drivers/gpu/drm/mediatek/mtk_dpi_regs.h |  3 +++
>  2 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
> b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index 9e4250356342..438bf3bc5e4a 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -128,6 +128,7 @@ struct mtk_dpi_yc_limit {
>   * @num_output_fmts: Quantity of supported output formats.
>   * @is_ck_de_pol: Support CK/DE polarity.
>   * @swap_input_support: Support input swap function.
> + * @color_fmt_trans_support: Enable color format transfer.
>   * @dimension_mask: Mask used for HWIDTH, HPORCH, VSYNC_WIDTH and
> VSYNC_PORCH
>   *		    (no shift).
>   * @hvsize_mask: Mask of HSIZE and VSIZE mask (no shift).
> @@ -144,6 +145,7 @@ struct mtk_dpi_conf {
>  	u32 num_output_fmts;
>  	bool is_ck_de_pol;
>  	bool swap_input_support;
> +	bool color_fmt_trans_support;
>  	u32 dimension_mask;
>  	u32 hvsize_mask;
>  	u32 channel_swap_shift;
> @@ -412,6 +414,31 @@ static void mtk_dpi_config_disable_edge(struct
> mtk_dpi *dpi)
>  		mtk_dpi_mask(dpi, dpi->conf->reg_h_fre_con, 0,
> EDGE_SEL_EN);
>  }
>  
> +static void mtk_dpi_matrix_sel(struct mtk_dpi *dpi,
> +			       enum mtk_dpi_out_color_format format)
> +{
> +	u32 matrix_sel = 0;
> +
> +	if (!dpi->conf->color_fmt_trans_support) {
> +		dev_info(dpi->dev, "matrix_sel is not supported.\n");
> +		return;
> +	}
> +
> +	switch (format) {
> +	case MTK_DPI_COLOR_FORMAT_YCBCR_422:
> +	case MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL:
> +	case MTK_DPI_COLOR_FORMAT_YCBCR_444:
> +	case MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL:

I think the transform formula are different for full range and non-full 
range. Please make sure '0x2' is for full range or non-full range. If
you are not sure, you could provide the transform matrix of '0x2' so we
could find out it's full or non-full.

> +	case MTK_DPI_COLOR_FORMAT_XV_YCC:
> +		if (dpi->mode.hdisplay <= 720)
> +			matrix_sel = 0x2;

Symbolize '0x2'.

> +		break;
> +	default:
> +		break;
> +	}
> +	mtk_dpi_mask(dpi, DPI_MATRIX_SET, matrix_sel,
> INT_MATRIX_SEL_MASK);
> +}
> +
>  static void mtk_dpi_config_color_format(struct mtk_dpi *dpi,
>  					enum mtk_dpi_out_color_format
> format)
>  {
> @@ -419,6 +446,7 @@ static void mtk_dpi_config_color_format(struct
> mtk_dpi *dpi,
>  	    (format == MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL)) {
>  		mtk_dpi_config_yuv422_enable(dpi, false);
>  		mtk_dpi_config_csc_enable(dpi, true);
> +		mtk_dpi_matrix_sel(dpi, format);

Why mt8173 support MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL but it does not
call mtk_dpi_matrix_sel()? It seems that mt8173 also need to call
mtk_dpi_matrix_sel() but lost and this patch looks like a bug fix for
all SoC DPI driver.

Regards,
CK

>  		if (dpi->conf->swap_input_support)
>  			mtk_dpi_config_swap_input(dpi, false);
>  		mtk_dpi_config_channel_swap(dpi,
> MTK_DPI_OUT_CHANNEL_SWAP_BGR);
> @@ -426,6 +454,7 @@ static void mtk_dpi_config_color_format(struct
> mtk_dpi *dpi,
>  		   (format == MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL)) {
>  		mtk_dpi_config_yuv422_enable(dpi, true);
>  		mtk_dpi_config_csc_enable(dpi, true);
> +		mtk_dpi_matrix_sel(dpi, format);
>  		if (dpi->conf->swap_input_support)
>  			mtk_dpi_config_swap_input(dpi, true);
>  		else
> @@ -673,7 +702,10 @@ static int mtk_dpi_bridge_atomic_check(struct
> drm_bridge *bridge,
>  	dpi->bit_num = MTK_DPI_OUT_BIT_NUM_8BITS;
>  	dpi->channel_swap = MTK_DPI_OUT_CHANNEL_SWAP_RGB;
>  	dpi->yc_map = MTK_DPI_OUT_YC_MAP_RGB;
> -	dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;
> +	if (out_bus_format == MEDIA_BUS_FMT_YUYV8_1X16)
> +		dpi->color_format =
> MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL;
> +	else
> +		dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> index 3a02fabe1662..cca0dccb84a2 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> @@ -217,4 +217,7 @@
>  
>  #define EDGE_SEL_EN			BIT(5)
>  #define H_FRE_2N			BIT(25)
> +
> +#define DPI_MATRIX_SET		0xB4
> +#define INT_MATRIX_SEL_MASK		GENMASK(4, 0)
>  #endif /* __MTK_DPI_REGS_H */
Rex-BC Chen (陳柏辰) June 28, 2022, 2:28 a.m. UTC | #2 
On Tue, 2022-06-28 at 10:15 +0800, CK Hu wrote:
> Hi, Bo-Chen:
> 
> On Fri, 2022-06-24 at 11:09 +0800, Bo-Chen Chen wrote:
> > Dp_intf supports YUV422 as output format. In MT8195 Chrome project,
> > YUV422 output format is used for 4K resolution.
> > 
> > To support this, it is also needed to support color format
> > transfer.
> > Color format transfer is a new feature for both dpi and dpintf of
> > MT8195.
> > 
> > The input format could be RGB888 and output format for dp_intf
> > should
> > be
> > YUV422. Therefore, we add a mtk_dpi_matrix_sel() helper to update
> > the
> > DPI_MATRIX_SET register depending on the color format.
> > 
> > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
> > Signed-off-by: Bo-Chen Chen <rex-bc.chen@mediatek.com>
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_dpi.c      | 34
> > ++++++++++++++++++++++++-
> >  drivers/gpu/drm/mediatek/mtk_dpi_regs.h |  3 +++
> >  2 files changed, 36 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > index 9e4250356342..438bf3bc5e4a 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > @@ -128,6 +128,7 @@ struct mtk_dpi_yc_limit {
> >   * @num_output_fmts: Quantity of supported output formats.
> >   * @is_ck_de_pol: Support CK/DE polarity.
> >   * @swap_input_support: Support input swap function.
> > + * @color_fmt_trans_support: Enable color format transfer.
> >   * @dimension_mask: Mask used for HWIDTH, HPORCH, VSYNC_WIDTH and
> > VSYNC_PORCH
> >   *		    (no shift).
> >   * @hvsize_mask: Mask of HSIZE and VSIZE mask (no shift).
> > @@ -144,6 +145,7 @@ struct mtk_dpi_conf {
> >  	u32 num_output_fmts;
> >  	bool is_ck_de_pol;
> >  	bool swap_input_support;
> > +	bool color_fmt_trans_support;
> >  	u32 dimension_mask;
> >  	u32 hvsize_mask;
> >  	u32 channel_swap_shift;
> > @@ -412,6 +414,31 @@ static void mtk_dpi_config_disable_edge(struct
> > mtk_dpi *dpi)
> >  		mtk_dpi_mask(dpi, dpi->conf->reg_h_fre_con, 0,
> > EDGE_SEL_EN);
> >  }
> >  
> > +static void mtk_dpi_matrix_sel(struct mtk_dpi *dpi,
> > +			       enum mtk_dpi_out_color_format format)
> > +{
> > +	u32 matrix_sel = 0;
> > +
> > +	if (!dpi->conf->color_fmt_trans_support) {
> > +		dev_info(dpi->dev, "matrix_sel is not supported.\n");
> > +		return;
> > +	}
> > +
> > +	switch (format) {
> > +	case MTK_DPI_COLOR_FORMAT_YCBCR_422:
> > +	case MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL:
> > +	case MTK_DPI_COLOR_FORMAT_YCBCR_444:
> > +	case MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL:
> 
> I think the transform formula are different for full range and non-
> full 
> range. Please make sure '0x2' is for full range or non-full range. If
> you are not sure, you could provide the transform matrix of '0x2' so
> we
> could find out it's full or non-full.
> 
> > +	case MTK_DPI_COLOR_FORMAT_XV_YCC:
> > +		if (dpi->mode.hdisplay <= 720)
> > +			matrix_sel = 0x2;
> 
> Symbolize '0x2'.
> 
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +	mtk_dpi_mask(dpi, DPI_MATRIX_SET, matrix_sel,
> > INT_MATRIX_SEL_MASK);
> > +}
> > +
> >  static void mtk_dpi_config_color_format(struct mtk_dpi *dpi,
> >  					enum mtk_dpi_out_color_format
> > format)
> >  {
> > @@ -419,6 +446,7 @@ static void mtk_dpi_config_color_format(struct
> > mtk_dpi *dpi,
> >  	    (format == MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL)) {
> >  		mtk_dpi_config_yuv422_enable(dpi, false);
> >  		mtk_dpi_config_csc_enable(dpi, true);
> > +		mtk_dpi_matrix_sel(dpi, format);
> 
> Why mt8173 support MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL but it does
> not
> call mtk_dpi_matrix_sel()? It seems that mt8173 also need to call
> mtk_dpi_matrix_sel() but lost and this patch looks like a bug fix for
> all SoC DPI driver.
> 
> Regards,
> CK
> 

Hello CK,

MT8173 does not support MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL as output
format, the output format is:

static const u32 mt8173_output_fmts[] = {
	MEDIA_BUS_FMT_RGB888_1X24,
};

or do I misunderstand?

BRs,
Bo-Chen

> >  		if (dpi->conf->swap_input_support)
> >  			mtk_dpi_config_swap_input(dpi, false);
> >  		mtk_dpi_config_channel_swap(dpi,
> > MTK_DPI_OUT_CHANNEL_SWAP_BGR);
> > @@ -426,6 +454,7 @@ static void mtk_dpi_config_color_format(struct
> > mtk_dpi *dpi,
> >  		   (format == MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL)) {
> >  		mtk_dpi_config_yuv422_enable(dpi, true);
> >  		mtk_dpi_config_csc_enable(dpi, true);
> > +		mtk_dpi_matrix_sel(dpi, format);
> >  		if (dpi->conf->swap_input_support)
> >  			mtk_dpi_config_swap_input(dpi, true);
> >  		else
> > @@ -673,7 +702,10 @@ static int mtk_dpi_bridge_atomic_check(struct
> > drm_bridge *bridge,
> >  	dpi->bit_num = MTK_DPI_OUT_BIT_NUM_8BITS;
> >  	dpi->channel_swap = MTK_DPI_OUT_CHANNEL_SWAP_RGB;
> >  	dpi->yc_map = MTK_DPI_OUT_YC_MAP_RGB;
> > -	dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;
> > +	if (out_bus_format == MEDIA_BUS_FMT_YUYV8_1X16)
> > +		dpi->color_format =
> > MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL;
> > +	else
> > +		dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;
> >  
> >  	return 0;
> >  }
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > index 3a02fabe1662..cca0dccb84a2 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > @@ -217,4 +217,7 @@
> >  
> >  #define EDGE_SEL_EN			BIT(5)
> >  #define H_FRE_2N			BIT(25)
> > +
> > +#define DPI_MATRIX_SET		0xB4
> > +#define INT_MATRIX_SEL_MASK		GENMASK(4, 0)
> >  #endif /* __MTK_DPI_REGS_H */
> 
>
CK Hu (胡俊光) June 28, 2022, 2:38 a.m. UTC | #3 
On Tue, 2022-06-28 at 10:28 +0800, Rex-BC Chen wrote:
> On Tue, 2022-06-28 at 10:15 +0800, CK Hu wrote:
> > Hi, Bo-Chen:
> > 
> > On Fri, 2022-06-24 at 11:09 +0800, Bo-Chen Chen wrote:
> > > Dp_intf supports YUV422 as output format. In MT8195 Chrome
> > > project,
> > > YUV422 output format is used for 4K resolution.
> > > 
> > > To support this, it is also needed to support color format
> > > transfer.
> > > Color format transfer is a new feature for both dpi and dpintf of
> > > MT8195.
> > > 
> > > The input format could be RGB888 and output format for dp_intf
> > > should
> > > be
> > > YUV422. Therefore, we add a mtk_dpi_matrix_sel() helper to update
> > > the
> > > DPI_MATRIX_SET register depending on the color format.
> > > 
> > > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
> > > Signed-off-by: Bo-Chen Chen <rex-bc.chen@mediatek.com>
> > > Reviewed-by: AngeloGioacchino Del Regno <
> > > angelogioacchino.delregno@collabora.com>
> > > ---
> > >  drivers/gpu/drm/mediatek/mtk_dpi.c      | 34
> > > ++++++++++++++++++++++++-
> > >  drivers/gpu/drm/mediatek/mtk_dpi_regs.h |  3 +++
> > >  2 files changed, 36 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > index 9e4250356342..438bf3bc5e4a 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > @@ -128,6 +128,7 @@ struct mtk_dpi_yc_limit {
> > >   * @num_output_fmts: Quantity of supported output formats.
> > >   * @is_ck_de_pol: Support CK/DE polarity.
> > >   * @swap_input_support: Support input swap function.
> > > + * @color_fmt_trans_support: Enable color format transfer.
> > >   * @dimension_mask: Mask used for HWIDTH, HPORCH, VSYNC_WIDTH
> > > and
> > > VSYNC_PORCH
> > >   *		    (no shift).
> > >   * @hvsize_mask: Mask of HSIZE and VSIZE mask (no shift).
> > > @@ -144,6 +145,7 @@ struct mtk_dpi_conf {
> > >  	u32 num_output_fmts;
> > >  	bool is_ck_de_pol;
> > >  	bool swap_input_support;
> > > +	bool color_fmt_trans_support;
> > >  	u32 dimension_mask;
> > >  	u32 hvsize_mask;
> > >  	u32 channel_swap_shift;
> > > @@ -412,6 +414,31 @@ static void
> > > mtk_dpi_config_disable_edge(struct
> > > mtk_dpi *dpi)
> > >  		mtk_dpi_mask(dpi, dpi->conf->reg_h_fre_con, 0,
> > > EDGE_SEL_EN);
> > >  }
> > >  
> > > +static void mtk_dpi_matrix_sel(struct mtk_dpi *dpi,
> > > +			       enum mtk_dpi_out_color_format format)
> > > +{
> > > +	u32 matrix_sel = 0;
> > > +
> > > +	if (!dpi->conf->color_fmt_trans_support) {
> > > +		dev_info(dpi->dev, "matrix_sel is not supported.\n");
> > > +		return;
> > > +	}
> > > +
> > > +	switch (format) {
> > > +	case MTK_DPI_COLOR_FORMAT_YCBCR_422:
> > > +	case MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL:
> > > +	case MTK_DPI_COLOR_FORMAT_YCBCR_444:
> > > +	case MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL:
> > 
> > I think the transform formula are different for full range and non-
> > full 
> > range. Please make sure '0x2' is for full range or non-full range.
> > If
> > you are not sure, you could provide the transform matrix of '0x2'
> > so
> > we
> > could find out it's full or non-full.
> > 
> > > +	case MTK_DPI_COLOR_FORMAT_XV_YCC:
> > > +		if (dpi->mode.hdisplay <= 720)
> > > +			matrix_sel = 0x2;
> > 
> > Symbolize '0x2'.
> > 
> > > +		break;
> > > +	default:
> > > +		break;
> > > +	}
> > > +	mtk_dpi_mask(dpi, DPI_MATRIX_SET, matrix_sel,
> > > INT_MATRIX_SEL_MASK);
> > > +}
> > > +
> > >  static void mtk_dpi_config_color_format(struct mtk_dpi *dpi,
> > >  					enum mtk_dpi_out_color_format
> > > format)
> > >  {
> > > @@ -419,6 +446,7 @@ static void
> > > mtk_dpi_config_color_format(struct
> > > mtk_dpi *dpi,
> > >  	    (format == MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL)) {
> > >  		mtk_dpi_config_yuv422_enable(dpi, false);
> > >  		mtk_dpi_config_csc_enable(dpi, true);
> > > +		mtk_dpi_matrix_sel(dpi, format);
> > 
> > Why mt8173 support MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL but it does
> > not
> > call mtk_dpi_matrix_sel()? It seems that mt8173 also need to call
> > mtk_dpi_matrix_sel() but lost and this patch looks like a bug fix
> > for
> > all SoC DPI driver.
> > 
> > Regards,
> > CK
> > 
> 
> Hello CK,
> 
> MT8173 does not support MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL as output
> format, the output format is:
> 
> static const u32 mt8173_output_fmts[] = {
> 	MEDIA_BUS_FMT_RGB888_1X24,
> };
> 
> or do I misunderstand?

In the first patch [1], it define

+enum mtk_dpi_out_color_format {
+	MTK_DPI_COLOR_FORMAT_RGB,
+	MTK_DPI_COLOR_FORMAT_RGB_FULL,
+	MTK_DPI_COLOR_FORMAT_YCBCR_444,
+	MTK_DPI_COLOR_FORMAT_YCBCR_422,
+	MTK_DPI_COLOR_FORMAT_XV_YCC,
+	MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL,
+	MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL
+};

and this function also process MTK_DPI_COLOR_FORMAT_YCBCR_444,
MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL, MTK_DPI_COLOR_FORMAT_YCBCR_422,
and MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL. So I think it want to process
output YUV but the caller of mtk_dpi_config_color_format() just pass
RGB into this function. If mt8173 does not support YUV output, I think
you should remove YUV processing in this function first, and then add
back YUV processing in this function.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek/mtk_dpi.c?h=v5.19-rc4&id=9e629c17aa8d7a75b8c1d99ed42892cd8ba7cdc4

Regards,
CK

> 
> BRs,
> Bo-Chen
> 
> > >  		if (dpi->conf->swap_input_support)
> > >  			mtk_dpi_config_swap_input(dpi, false);
> > >  		mtk_dpi_config_channel_swap(dpi,
> > > MTK_DPI_OUT_CHANNEL_SWAP_BGR);
> > > @@ -426,6 +454,7 @@ static void
> > > mtk_dpi_config_color_format(struct
> > > mtk_dpi *dpi,
> > >  		   (format == MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL)) {
> > >  		mtk_dpi_config_yuv422_enable(dpi, true);
> > >  		mtk_dpi_config_csc_enable(dpi, true);
> > > +		mtk_dpi_matrix_sel(dpi, format);
> > >  		if (dpi->conf->swap_input_support)
> > >  			mtk_dpi_config_swap_input(dpi, true);
> > >  		else
> > > @@ -673,7 +702,10 @@ static int
> > > mtk_dpi_bridge_atomic_check(struct
> > > drm_bridge *bridge,
> > >  	dpi->bit_num = MTK_DPI_OUT_BIT_NUM_8BITS;
> > >  	dpi->channel_swap = MTK_DPI_OUT_CHANNEL_SWAP_RGB;
> > >  	dpi->yc_map = MTK_DPI_OUT_YC_MAP_RGB;
> > > -	dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;
> > > +	if (out_bus_format == MEDIA_BUS_FMT_YUYV8_1X16)
> > > +		dpi->color_format =
> > > MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL;
> > > +	else
> > > +		dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;
> > >  
> > >  	return 0;
> > >  }
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > > b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > > index 3a02fabe1662..cca0dccb84a2 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > > +++ b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > > @@ -217,4 +217,7 @@
> > >  
> > >  #define EDGE_SEL_EN			BIT(5)
> > >  #define H_FRE_2N			BIT(25)
> > > +
> > > +#define DPI_MATRIX_SET		0xB4
> > > +#define INT_MATRIX_SEL_MASK		GENMASK(4, 0)
> > >  #endif /* __MTK_DPI_REGS_H */
> > 
> > 
> 
>
Rex-BC Chen (陳柏辰) June 28, 2022, 3:01 a.m. UTC | #4 
On Tue, 2022-06-28 at 10:38 +0800, CK Hu wrote:
> On Tue, 2022-06-28 at 10:28 +0800, Rex-BC Chen wrote:
> > On Tue, 2022-06-28 at 10:15 +0800, CK Hu wrote:
> > > Hi, Bo-Chen:
> > > 
> > > On Fri, 2022-06-24 at 11:09 +0800, Bo-Chen Chen wrote:
> > > > Dp_intf supports YUV422 as output format. In MT8195 Chrome
> > > > project,
> > > > YUV422 output format is used for 4K resolution.
> > > > 
> > > > To support this, it is also needed to support color format
> > > > transfer.
> > > > Color format transfer is a new feature for both dpi and dpintf
> > > > of
> > > > MT8195.
> > > > 
> > > > The input format could be RGB888 and output format for dp_intf
> > > > should
> > > > be
> > > > YUV422. Therefore, we add a mtk_dpi_matrix_sel() helper to
> > > > update
> > > > the
> > > > DPI_MATRIX_SET register depending on the color format.
> > > > 
> > > > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
> > > > Signed-off-by: Bo-Chen Chen <rex-bc.chen@mediatek.com>
> > > > Reviewed-by: AngeloGioacchino Del Regno <
> > > > angelogioacchino.delregno@collabora.com>
> > > > ---
> > > >  drivers/gpu/drm/mediatek/mtk_dpi.c      | 34
> > > > ++++++++++++++++++++++++-
> > > >  drivers/gpu/drm/mediatek/mtk_dpi_regs.h |  3 +++
> > > >  2 files changed, 36 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > > b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > > index 9e4250356342..438bf3bc5e4a 100644
> > > > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > > @@ -128,6 +128,7 @@ struct mtk_dpi_yc_limit {
> > > >   * @num_output_fmts: Quantity of supported output formats.
> > > >   * @is_ck_de_pol: Support CK/DE polarity.
> > > >   * @swap_input_support: Support input swap function.
> > > > + * @color_fmt_trans_support: Enable color format transfer.
> > > >   * @dimension_mask: Mask used for HWIDTH, HPORCH, VSYNC_WIDTH
> > > > and
> > > > VSYNC_PORCH
> > > >   *		    (no shift).
> > > >   * @hvsize_mask: Mask of HSIZE and VSIZE mask (no shift).
> > > > @@ -144,6 +145,7 @@ struct mtk_dpi_conf {
> > > >  	u32 num_output_fmts;
> > > >  	bool is_ck_de_pol;
> > > >  	bool swap_input_support;
> > > > +	bool color_fmt_trans_support;
> > > >  	u32 dimension_mask;
> > > >  	u32 hvsize_mask;
> > > >  	u32 channel_swap_shift;
> > > > @@ -412,6 +414,31 @@ static void
> > > > mtk_dpi_config_disable_edge(struct
> > > > mtk_dpi *dpi)
> > > >  		mtk_dpi_mask(dpi, dpi->conf->reg_h_fre_con, 0,
> > > > EDGE_SEL_EN);
> > > >  }
> > > >  
> > > > +static void mtk_dpi_matrix_sel(struct mtk_dpi *dpi,
> > > > +			       enum mtk_dpi_out_color_format
> > > > format)
> > > > +{
> > > > +	u32 matrix_sel = 0;
> > > > +
> > > > +	if (!dpi->conf->color_fmt_trans_support) {
> > > > +		dev_info(dpi->dev, "matrix_sel is not
> > > > supported.\n");
> > > > +		return;
> > > > +	}
> > > > +
> > > > +	switch (format) {
> > > > +	case MTK_DPI_COLOR_FORMAT_YCBCR_422:
> > > > +	case MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL:
> > > > +	case MTK_DPI_COLOR_FORMAT_YCBCR_444:
> > > > +	case MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL:
> > > 
> > > I think the transform formula are different for full range and
> > > non-
> > > full 
> > > range. Please make sure '0x2' is for full range or non-full
> > > range.
> > > If
> > > you are not sure, you could provide the transform matrix of '0x2'
> > > so
> > > we
> > > could find out it's full or non-full.
> > > 
> > > > +	case MTK_DPI_COLOR_FORMAT_XV_YCC:
> > > > +		if (dpi->mode.hdisplay <= 720)
> > > > +			matrix_sel = 0x2;
> > > 
> > > Symbolize '0x2'.
> > > 
> > > > +		break;
> > > > +	default:
> > > > +		break;
> > > > +	}
> > > > +	mtk_dpi_mask(dpi, DPI_MATRIX_SET, matrix_sel,
> > > > INT_MATRIX_SEL_MASK);
> > > > +}
> > > > +
> > > >  static void mtk_dpi_config_color_format(struct mtk_dpi *dpi,
> > > >  					enum
> > > > mtk_dpi_out_color_format
> > > > format)
> > > >  {
> > > > @@ -419,6 +446,7 @@ static void
> > > > mtk_dpi_config_color_format(struct
> > > > mtk_dpi *dpi,
> > > >  	    (format == MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL)) {
> > > >  		mtk_dpi_config_yuv422_enable(dpi, false);
> > > >  		mtk_dpi_config_csc_enable(dpi, true);
> > > > +		mtk_dpi_matrix_sel(dpi, format);
> > > 
> > > Why mt8173 support MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL but it
> > > does
> > > not
> > > call mtk_dpi_matrix_sel()? It seems that mt8173 also need to call
> > > mtk_dpi_matrix_sel() but lost and this patch looks like a bug fix
> > > for
> > > all SoC DPI driver.
> > > 
> > > Regards,
> > > CK
> > > 
> > 
> > Hello CK,
> > 
> > MT8173 does not support MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL as
> > output
> > format, the output format is:
> > 
> > static const u32 mt8173_output_fmts[] = {
> > 	MEDIA_BUS_FMT_RGB888_1X24,
> > };
> > 
> > or do I misunderstand?
> 
> In the first patch [1], it define
> 
> +enum mtk_dpi_out_color_format {
> +	MTK_DPI_COLOR_FORMAT_RGB,
> +	MTK_DPI_COLOR_FORMAT_RGB_FULL,
> +	MTK_DPI_COLOR_FORMAT_YCBCR_444,
> +	MTK_DPI_COLOR_FORMAT_YCBCR_422,
> +	MTK_DPI_COLOR_FORMAT_XV_YCC,
> +	MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL,
> +	MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL
> +};
> 
> and this function also process MTK_DPI_COLOR_FORMAT_YCBCR_444,
> MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL, MTK_DPI_COLOR_FORMAT_YCBCR_422,
> and MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL. So I think it want to
> process
> output YUV but the caller of mtk_dpi_config_color_format() just pass
> RGB into this function. If mt8173 does not support YUV output, I
> think
> you should remove YUV processing in this function first, and then add
> back YUV processing in this function.
> 
> [1] 
> 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek/mtk_dpi.c?h=v5.19-rc4&id=9e629c17aa8d7a75b8c1d99ed42892cd8ba7cdc4
> 
> Regards,
> CK
> 

Hello CK,

I don't think it should be remove. After all, it is accepted and from
[1], we can see it assgin output format always as RGB.
+	dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;

After that, it also added patch of supporting changing output
format[2].

We have support output as YUV422 now. I think it's ok to just add this.
If we remove them and add them back, I think it is a little bit
redundant. And I also can add a commit message like "output format
YUV422 is not support for previous MediaTek SoCs. MT8195 supports
output format as YUV422.."

What do you think?

[2]: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek/mtk_dpi.c?h=v5.19-rc4&id=be63f6e8601ff21139da93623754717e92cbd8db

BRs,
Bo-Chen
> > 
> > BRs,
> > Bo-Chen
> > 
> > > >  		if (dpi->conf->swap_input_support)
> > > >  			mtk_dpi_config_swap_input(dpi, false);
> > > >  		mtk_dpi_config_channel_swap(dpi,
> > > > MTK_DPI_OUT_CHANNEL_SWAP_BGR);
> > > > @@ -426,6 +454,7 @@ static void
> > > > mtk_dpi_config_color_format(struct
> > > > mtk_dpi *dpi,
> > > >  		   (format ==
> > > > MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL)) {
> > > >  		mtk_dpi_config_yuv422_enable(dpi, true);
> > > >  		mtk_dpi_config_csc_enable(dpi, true);
> > > > +		mtk_dpi_matrix_sel(dpi, format);
> > > >  		if (dpi->conf->swap_input_support)
> > > >  			mtk_dpi_config_swap_input(dpi, true);
> > > >  		else
> > > > @@ -673,7 +702,10 @@ static int
> > > > mtk_dpi_bridge_atomic_check(struct
> > > > drm_bridge *bridge,
> > > >  	dpi->bit_num = MTK_DPI_OUT_BIT_NUM_8BITS;
> > > >  	dpi->channel_swap = MTK_DPI_OUT_CHANNEL_SWAP_RGB;
> > > >  	dpi->yc_map = MTK_DPI_OUT_YC_MAP_RGB;
> > > > -	dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;
> > > > +	if (out_bus_format == MEDIA_BUS_FMT_YUYV8_1X16)
> > > > +		dpi->color_format =
> > > > MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL;
> > > > +	else
> > > > +		dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;
> > > >  
> > > >  	return 0;
> > > >  }
> > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > > > b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > > > index 3a02fabe1662..cca0dccb84a2 100644
> > > > --- a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > > > +++ b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > > > @@ -217,4 +217,7 @@
> > > >  
> > > >  #define EDGE_SEL_EN			BIT(5)
> > > >  #define H_FRE_2N			BIT(25)
> > > > +
> > > > +#define DPI_MATRIX_SET		0xB4
> > > > +#define INT_MATRIX_SEL_MASK		GENMASK(4, 0)
> > > >  #endif /* __MTK_DPI_REGS_H */
> > > 
> > > 
> > 
> > 
> 
>
CK Hu (胡俊光) June 28, 2022, 3:23 a.m. UTC | #5 
On Tue, 2022-06-28 at 11:01 +0800, Rex-BC Chen wrote:
> On Tue, 2022-06-28 at 10:38 +0800, CK Hu wrote:
> > On Tue, 2022-06-28 at 10:28 +0800, Rex-BC Chen wrote:
> > > On Tue, 2022-06-28 at 10:15 +0800, CK Hu wrote:
> > > > Hi, Bo-Chen:
> > > > 
> > > > On Fri, 2022-06-24 at 11:09 +0800, Bo-Chen Chen wrote:
> > > > > Dp_intf supports YUV422 as output format. In MT8195 Chrome
> > > > > project,
> > > > > YUV422 output format is used for 4K resolution.
> > > > > 
> > > > > To support this, it is also needed to support color format
> > > > > transfer.
> > > > > Color format transfer is a new feature for both dpi and
> > > > > dpintf
> > > > > of
> > > > > MT8195.
> > > > > 
> > > > > The input format could be RGB888 and output format for
> > > > > dp_intf
> > > > > should
> > > > > be
> > > > > YUV422. Therefore, we add a mtk_dpi_matrix_sel() helper to
> > > > > update
> > > > > the
> > > > > DPI_MATRIX_SET register depending on the color format.
> > > > > 
> > > > > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
> > > > > Signed-off-by: Bo-Chen Chen <rex-bc.chen@mediatek.com>
> > > > > Reviewed-by: AngeloGioacchino Del Regno <
> > > > > angelogioacchino.delregno@collabora.com>
> > > > > ---
> > > > >  drivers/gpu/drm/mediatek/mtk_dpi.c      | 34
> > > > > ++++++++++++++++++++++++-
> > > > >  drivers/gpu/drm/mediatek/mtk_dpi_regs.h |  3 +++
> > > > >  2 files changed, 36 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > > > b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > > > index 9e4250356342..438bf3bc5e4a 100644
> > > > > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > > > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > > > @@ -128,6 +128,7 @@ struct mtk_dpi_yc_limit {
> > > > >   * @num_output_fmts: Quantity of supported output formats.
> > > > >   * @is_ck_de_pol: Support CK/DE polarity.
> > > > >   * @swap_input_support: Support input swap function.
> > > > > + * @color_fmt_trans_support: Enable color format transfer.
> > > > >   * @dimension_mask: Mask used for HWIDTH, HPORCH,
> > > > > VSYNC_WIDTH
> > > > > and
> > > > > VSYNC_PORCH
> > > > >   *		    (no shift).
> > > > >   * @hvsize_mask: Mask of HSIZE and VSIZE mask (no shift).
> > > > > @@ -144,6 +145,7 @@ struct mtk_dpi_conf {
> > > > >  	u32 num_output_fmts;
> > > > >  	bool is_ck_de_pol;
> > > > >  	bool swap_input_support;
> > > > > +	bool color_fmt_trans_support;
> > > > >  	u32 dimension_mask;
> > > > >  	u32 hvsize_mask;
> > > > >  	u32 channel_swap_shift;
> > > > > @@ -412,6 +414,31 @@ static void
> > > > > mtk_dpi_config_disable_edge(struct
> > > > > mtk_dpi *dpi)
> > > > >  		mtk_dpi_mask(dpi, dpi->conf->reg_h_fre_con, 0,
> > > > > EDGE_SEL_EN);
> > > > >  }
> > > > >  
> > > > > +static void mtk_dpi_matrix_sel(struct mtk_dpi *dpi,
> > > > > +			       enum mtk_dpi_out_color_format
> > > > > format)
> > > > > +{
> > > > > +	u32 matrix_sel = 0;
> > > > > +
> > > > > +	if (!dpi->conf->color_fmt_trans_support) {
> > > > > +		dev_info(dpi->dev, "matrix_sel is not
> > > > > supported.\n");
> > > > > +		return;
> > > > > +	}
> > > > > +
> > > > > +	switch (format) {
> > > > > +	case MTK_DPI_COLOR_FORMAT_YCBCR_422:
> > > > > +	case MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL:
> > > > > +	case MTK_DPI_COLOR_FORMAT_YCBCR_444:
> > > > > +	case MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL:
> > > > 
> > > > I think the transform formula are different for full range and
> > > > non-
> > > > full 
> > > > range. Please make sure '0x2' is for full range or non-full
> > > > range.
> > > > If
> > > > you are not sure, you could provide the transform matrix of
> > > > '0x2'
> > > > so
> > > > we
> > > > could find out it's full or non-full.
> > > > 
> > > > > +	case MTK_DPI_COLOR_FORMAT_XV_YCC:
> > > > > +		if (dpi->mode.hdisplay <= 720)
> > > > > +			matrix_sel = 0x2;
> > > > 
> > > > Symbolize '0x2'.
> > > > 
> > > > > +		break;
> > > > > +	default:
> > > > > +		break;
> > > > > +	}
> > > > > +	mtk_dpi_mask(dpi, DPI_MATRIX_SET, matrix_sel,
> > > > > INT_MATRIX_SEL_MASK);
> > > > > +}
> > > > > +
> > > > >  static void mtk_dpi_config_color_format(struct mtk_dpi *dpi,
> > > > >  					enum
> > > > > mtk_dpi_out_color_format
> > > > > format)
> > > > >  {
> > > > > @@ -419,6 +446,7 @@ static void
> > > > > mtk_dpi_config_color_format(struct
> > > > > mtk_dpi *dpi,
> > > > >  	    (format == MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL)) {
> > > > >  		mtk_dpi_config_yuv422_enable(dpi, false);
> > > > >  		mtk_dpi_config_csc_enable(dpi, true);
> > > > > +		mtk_dpi_matrix_sel(dpi, format);
> > > > 
> > > > Why mt8173 support MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL but it
> > > > does
> > > > not
> > > > call mtk_dpi_matrix_sel()? It seems that mt8173 also need to
> > > > call
> > > > mtk_dpi_matrix_sel() but lost and this patch looks like a bug
> > > > fix
> > > > for
> > > > all SoC DPI driver.
> > > > 
> > > > Regards,
> > > > CK
> > > > 
> > > 
> > > Hello CK,
> > > 
> > > MT8173 does not support MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL as
> > > output
> > > format, the output format is:
> > > 
> > > static const u32 mt8173_output_fmts[] = {
> > > 	MEDIA_BUS_FMT_RGB888_1X24,
> > > };
> > > 
> > > or do I misunderstand?
> > 
> > In the first patch [1], it define
> > 
> > +enum mtk_dpi_out_color_format {
> > +	MTK_DPI_COLOR_FORMAT_RGB,
> > +	MTK_DPI_COLOR_FORMAT_RGB_FULL,
> > +	MTK_DPI_COLOR_FORMAT_YCBCR_444,
> > +	MTK_DPI_COLOR_FORMAT_YCBCR_422,
> > +	MTK_DPI_COLOR_FORMAT_XV_YCC,
> > +	MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL,
> > +	MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL
> > +};
> > 
> > and this function also process MTK_DPI_COLOR_FORMAT_YCBCR_444,
> > MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL,
> > MTK_DPI_COLOR_FORMAT_YCBCR_422,
> > and MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL. So I think it want to
> > process
> > output YUV but the caller of mtk_dpi_config_color_format() just
> > pass
> > RGB into this function. If mt8173 does not support YUV output, I
> > think
> > you should remove YUV processing in this function first, and then
> > add
> > back YUV processing in this function.
> > 
> > [1] 
> > 
> 
> 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek/mtk_dpi.c?h=v5.19-rc4&id=9e629c17aa8d7a75b8c1d99ed42892cd8ba7cdc4
> > 
> > Regards,
> > CK
> > 
> 
> Hello CK,
> 
> I don't think it should be remove. After all, it is accepted and from
> [1], we can see it assgin output format always as RGB.
> +	dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;
> 
> After that, it also added patch of supporting changing output
> format[2].
> 
> We have support output as YUV422 now. I think it's ok to just add
> this.
> If we remove them and add them back, I think it is a little bit
> redundant. And I also can add a commit message like "output format
> YUV422 is not support for previous MediaTek SoCs. MT8195 supports
> output format as YUV422.."
> 
> What do you think?

If we have no information about mt8173 support YUV or not, we could
just treat it as not support YUV. I like to make things clean before
apply new feature. If you do not want to remove it, I would send a
patch to remove it.

Regards,
CK

> 
> [2]: 
> 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek/mtk_dpi.c?h=v5.19-rc4&id=be63f6e8601ff21139da93623754717e92cbd8db
> 
> BRs,
> Bo-Chen
> > > 
> > > BRs,
> > > Bo-Chen
> > > 
> > > > >  		if (dpi->conf->swap_input_support)
> > > > >  			mtk_dpi_config_swap_input(dpi, false);
> > > > >  		mtk_dpi_config_channel_swap(dpi,
> > > > > MTK_DPI_OUT_CHANNEL_SWAP_BGR);
> > > > > @@ -426,6 +454,7 @@ static void
> > > > > mtk_dpi_config_color_format(struct
> > > > > mtk_dpi *dpi,
> > > > >  		   (format ==
> > > > > MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL)) {
> > > > >  		mtk_dpi_config_yuv422_enable(dpi, true);
> > > > >  		mtk_dpi_config_csc_enable(dpi, true);
> > > > > +		mtk_dpi_matrix_sel(dpi, format);
> > > > >  		if (dpi->conf->swap_input_support)
> > > > >  			mtk_dpi_config_swap_input(dpi, true);
> > > > >  		else
> > > > > @@ -673,7 +702,10 @@ static int
> > > > > mtk_dpi_bridge_atomic_check(struct
> > > > > drm_bridge *bridge,
> > > > >  	dpi->bit_num = MTK_DPI_OUT_BIT_NUM_8BITS;
> > > > >  	dpi->channel_swap = MTK_DPI_OUT_CHANNEL_SWAP_RGB;
> > > > >  	dpi->yc_map = MTK_DPI_OUT_YC_MAP_RGB;
> > > > > -	dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;
> > > > > +	if (out_bus_format == MEDIA_BUS_FMT_YUYV8_1X16)
> > > > > +		dpi->color_format =
> > > > > MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL;
> > > > > +	else
> > > > > +		dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;
> > > > >  
> > > > >  	return 0;
> > > > >  }
> > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > > > > b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > > > > index 3a02fabe1662..cca0dccb84a2 100644
> > > > > --- a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > > > > +++ b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > > > > @@ -217,4 +217,7 @@
> > > > >  
> > > > >  #define EDGE_SEL_EN			BIT(5)
> > > > >  #define H_FRE_2N			BIT(25)
> > > > > +
> > > > > +#define DPI_MATRIX_SET		0xB4
> > > > > +#define INT_MATRIX_SEL_MASK		GENMASK(4, 0)
> > > > >  #endif /* __MTK_DPI_REGS_H */
> > > > 
> > > > 
> > > 
> > > 
> > 
> > 
> 
>
Rex-BC Chen (陳柏辰) June 28, 2022, 3:27 a.m. UTC | #6 
On Tue, 2022-06-28 at 11:23 +0800, CK Hu wrote:
> On Tue, 2022-06-28 at 11:01 +0800, Rex-BC Chen wrote:
> > On Tue, 2022-06-28 at 10:38 +0800, CK Hu wrote:
> > > On Tue, 2022-06-28 at 10:28 +0800, Rex-BC Chen wrote:
> > > > On Tue, 2022-06-28 at 10:15 +0800, CK Hu wrote:
> > > > > Hi, Bo-Chen:
> > > > > 
> > > > > On Fri, 2022-06-24 at 11:09 +0800, Bo-Chen Chen wrote:
> > > > > > Dp_intf supports YUV422 as output format. In MT8195 Chrome
> > > > > > project,
> > > > > > YUV422 output format is used for 4K resolution.
> > > > > > 
> > > > > > To support this, it is also needed to support color format
> > > > > > transfer.
> > > > > > Color format transfer is a new feature for both dpi and
> > > > > > dpintf
> > > > > > of
> > > > > > MT8195.
> > > > > > 
> > > > > > The input format could be RGB888 and output format for
> > > > > > dp_intf
> > > > > > should
> > > > > > be
> > > > > > YUV422. Therefore, we add a mtk_dpi_matrix_sel() helper to
> > > > > > update
> > > > > > the
> > > > > > DPI_MATRIX_SET register depending on the color format.
> > > > > > 
> > > > > > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
> > > > > > Signed-off-by: Bo-Chen Chen <rex-bc.chen@mediatek.com>
> > > > > > Reviewed-by: AngeloGioacchino Del Regno <
> > > > > > angelogioacchino.delregno@collabora.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/mediatek/mtk_dpi.c      | 34
> > > > > > ++++++++++++++++++++++++-
> > > > > >  drivers/gpu/drm/mediatek/mtk_dpi_regs.h |  3 +++
> > > > > >  2 files changed, 36 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > > > > b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > > > > index 9e4250356342..438bf3bc5e4a 100644
> > > > > > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > > > > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > > > > > @@ -128,6 +128,7 @@ struct mtk_dpi_yc_limit {
> > > > > >   * @num_output_fmts: Quantity of supported output formats.
> > > > > >   * @is_ck_de_pol: Support CK/DE polarity.
> > > > > >   * @swap_input_support: Support input swap function.
> > > > > > + * @color_fmt_trans_support: Enable color format transfer.
> > > > > >   * @dimension_mask: Mask used for HWIDTH, HPORCH,
> > > > > > VSYNC_WIDTH
> > > > > > and
> > > > > > VSYNC_PORCH
> > > > > >   *		    (no shift).
> > > > > >   * @hvsize_mask: Mask of HSIZE and VSIZE mask (no shift).
> > > > > > @@ -144,6 +145,7 @@ struct mtk_dpi_conf {
> > > > > >  	u32 num_output_fmts;
> > > > > >  	bool is_ck_de_pol;
> > > > > >  	bool swap_input_support;
> > > > > > +	bool color_fmt_trans_support;
> > > > > >  	u32 dimension_mask;
> > > > > >  	u32 hvsize_mask;
> > > > > >  	u32 channel_swap_shift;
> > > > > > @@ -412,6 +414,31 @@ static void
> > > > > > mtk_dpi_config_disable_edge(struct
> > > > > > mtk_dpi *dpi)
> > > > > >  		mtk_dpi_mask(dpi, dpi->conf->reg_h_fre_con, 0,
> > > > > > EDGE_SEL_EN);
> > > > > >  }
> > > > > >  
> > > > > > +static void mtk_dpi_matrix_sel(struct mtk_dpi *dpi,
> > > > > > +			       enum mtk_dpi_out_color_format
> > > > > > format)
> > > > > > +{
> > > > > > +	u32 matrix_sel = 0;
> > > > > > +
> > > > > > +	if (!dpi->conf->color_fmt_trans_support) {
> > > > > > +		dev_info(dpi->dev, "matrix_sel is not
> > > > > > supported.\n");
> > > > > > +		return;
> > > > > > +	}
> > > > > > +
> > > > > > +	switch (format) {
> > > > > > +	case MTK_DPI_COLOR_FORMAT_YCBCR_422:
> > > > > > +	case MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL:
> > > > > > +	case MTK_DPI_COLOR_FORMAT_YCBCR_444:
> > > > > > +	case MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL:
> > > > > 
> > > > > I think the transform formula are different for full range
> > > > > and
> > > > > non-
> > > > > full 
> > > > > range. Please make sure '0x2' is for full range or non-full
> > > > > range.
> > > > > If
> > > > > you are not sure, you could provide the transform matrix of
> > > > > '0x2'
> > > > > so
> > > > > we
> > > > > could find out it's full or non-full.
> > > > > 
> > > > > > +	case MTK_DPI_COLOR_FORMAT_XV_YCC:
> > > > > > +		if (dpi->mode.hdisplay <= 720)
> > > > > > +			matrix_sel = 0x2;
> > > > > 
> > > > > Symbolize '0x2'.
> > > > > 
> > > > > > +		break;
> > > > > > +	default:
> > > > > > +		break;
> > > > > > +	}
> > > > > > +	mtk_dpi_mask(dpi, DPI_MATRIX_SET, matrix_sel,
> > > > > > INT_MATRIX_SEL_MASK);
> > > > > > +}
> > > > > > +
> > > > > >  static void mtk_dpi_config_color_format(struct mtk_dpi
> > > > > > *dpi,
> > > > > >  					enum
> > > > > > mtk_dpi_out_color_format
> > > > > > format)
> > > > > >  {
> > > > > > @@ -419,6 +446,7 @@ static void
> > > > > > mtk_dpi_config_color_format(struct
> > > > > > mtk_dpi *dpi,
> > > > > >  	    (format == MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL)) {
> > > > > >  		mtk_dpi_config_yuv422_enable(dpi, false);
> > > > > >  		mtk_dpi_config_csc_enable(dpi, true);
> > > > > > +		mtk_dpi_matrix_sel(dpi, format);
> > > > > 
> > > > > Why mt8173 support MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL but it
> > > > > does
> > > > > not
> > > > > call mtk_dpi_matrix_sel()? It seems that mt8173 also need to
> > > > > call
> > > > > mtk_dpi_matrix_sel() but lost and this patch looks like a bug
> > > > > fix
> > > > > for
> > > > > all SoC DPI driver.
> > > > > 
> > > > > Regards,
> > > > > CK
> > > > > 
> > > > 
> > > > Hello CK,
> > > > 
> > > > MT8173 does not support MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL as
> > > > output
> > > > format, the output format is:
> > > > 
> > > > static const u32 mt8173_output_fmts[] = {
> > > > 	MEDIA_BUS_FMT_RGB888_1X24,
> > > > };
> > > > 
> > > > or do I misunderstand?
> > > 
> > > In the first patch [1], it define
> > > 
> > > +enum mtk_dpi_out_color_format {
> > > +	MTK_DPI_COLOR_FORMAT_RGB,
> > > +	MTK_DPI_COLOR_FORMAT_RGB_FULL,
> > > +	MTK_DPI_COLOR_FORMAT_YCBCR_444,
> > > +	MTK_DPI_COLOR_FORMAT_YCBCR_422,
> > > +	MTK_DPI_COLOR_FORMAT_XV_YCC,
> > > +	MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL,
> > > +	MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL
> > > +};
> > > 
> > > and this function also process MTK_DPI_COLOR_FORMAT_YCBCR_444,
> > > MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL,
> > > MTK_DPI_COLOR_FORMAT_YCBCR_422,
> > > and MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL. So I think it want to
> > > process
> > > output YUV but the caller of mtk_dpi_config_color_format() just
> > > pass
> > > RGB into this function. If mt8173 does not support YUV output, I
> > > think
> > > you should remove YUV processing in this function first, and then
> > > add
> > > back YUV processing in this function.
> > > 
> > > [1] 
> > > 
> > 
> > 
> 
> 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek/mtk_dpi.c?h=v5.19-rc4&id=9e629c17aa8d7a75b8c1d99ed42892cd8ba7cdc4
> > > 
> > > Regards,
> > > CK
> > > 
> > 
> > Hello CK,
> > 
> > I don't think it should be remove. After all, it is accepted and
> > from
> > [1], we can see it assgin output format always as RGB.
> > +	dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;
> > 
> > After that, it also added patch of supporting changing output
> > format[2].
> > 
> > We have support output as YUV422 now. I think it's ok to just add
> > this.
> > If we remove them and add them back, I think it is a little bit
> > redundant. And I also can add a commit message like "output format
> > YUV422 is not support for previous MediaTek SoCs. MT8195 supports
> > output format as YUV422.."
> > 
> > What do you think?
> 
> If we have no information about mt8173 support YUV or not, we could
> just treat it as not support YUV. I like to make things clean before
> apply new feature. If you do not want to remove it, I would send a
> patch to remove it.
> 
> Regards,
> CK
> 

Hello CK,

ok, I will confirm whether 8173 support yuv422 as output. If no
answerd, I will also remove them in next version of this series.

Thanks.

BRs,
Bo-Chen

> > 
> > [2]: 
> > 
> 
> 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek/mtk_dpi.c?h=v5.19-rc4&id=be63f6e8601ff21139da93623754717e92cbd8db
> > 
> > BRs,
> > Bo-Chen
> > > > 
> > > > BRs,
> > > > Bo-Chen
> > > > 
> > > > > >  		if (dpi->conf->swap_input_support)
> > > > > >  			mtk_dpi_config_swap_input(dpi, false);
> > > > > >  		mtk_dpi_config_channel_swap(dpi,
> > > > > > MTK_DPI_OUT_CHANNEL_SWAP_BGR);
> > > > > > @@ -426,6 +454,7 @@ static void
> > > > > > mtk_dpi_config_color_format(struct
> > > > > > mtk_dpi *dpi,
> > > > > >  		   (format ==
> > > > > > MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL)) {
> > > > > >  		mtk_dpi_config_yuv422_enable(dpi, true);
> > > > > >  		mtk_dpi_config_csc_enable(dpi, true);
> > > > > > +		mtk_dpi_matrix_sel(dpi, format);
> > > > > >  		if (dpi->conf->swap_input_support)
> > > > > >  			mtk_dpi_config_swap_input(dpi, true);
> > > > > >  		else
> > > > > > @@ -673,7 +702,10 @@ static int
> > > > > > mtk_dpi_bridge_atomic_check(struct
> > > > > > drm_bridge *bridge,
> > > > > >  	dpi->bit_num = MTK_DPI_OUT_BIT_NUM_8BITS;
> > > > > >  	dpi->channel_swap = MTK_DPI_OUT_CHANNEL_SWAP_RGB;
> > > > > >  	dpi->yc_map = MTK_DPI_OUT_YC_MAP_RGB;
> > > > > > -	dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;
> > > > > > +	if (out_bus_format == MEDIA_BUS_FMT_YUYV8_1X16)
> > > > > > +		dpi->color_format =
> > > > > > MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL;
> > > > > > +	else
> > > > > > +		dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;
> > > > > >  
> > > > > >  	return 0;
> > > > > >  }
> > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > > > > > b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > > > > > index 3a02fabe1662..cca0dccb84a2 100644
> > > > > > --- a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > > > > > +++ b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> > > > > > @@ -217,4 +217,7 @@
> > > > > >  
> > > > > >  #define EDGE_SEL_EN			BIT(5)
> > > > > >  #define H_FRE_2N			BIT(25)
> > > > > > +
> > > > > > +#define DPI_MATRIX_SET		0xB4
> > > > > > +#define INT_MATRIX_SEL_MASK		GENMASK(4, 0)
> > > > > >  #endif /* __MTK_DPI_REGS_H */
> > > > > 
> > > > > 
> > > > 
> > > > 
> > > 
> > > 
> > 
> > 
> 
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 9e4250356342..438bf3bc5e4a 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -128,6 +128,7 @@  struct mtk_dpi_yc_limit {
  * @num_output_fmts: Quantity of supported output formats.
  * @is_ck_de_pol: Support CK/DE polarity.
  * @swap_input_support: Support input swap function.
+ * @color_fmt_trans_support: Enable color format transfer.
  * @dimension_mask: Mask used for HWIDTH, HPORCH, VSYNC_WIDTH and VSYNC_PORCH
  *		    (no shift).
  * @hvsize_mask: Mask of HSIZE and VSIZE mask (no shift).
@@ -144,6 +145,7 @@  struct mtk_dpi_conf {
 	u32 num_output_fmts;
 	bool is_ck_de_pol;
 	bool swap_input_support;
+	bool color_fmt_trans_support;
 	u32 dimension_mask;
 	u32 hvsize_mask;
 	u32 channel_swap_shift;
@@ -412,6 +414,31 @@  static void mtk_dpi_config_disable_edge(struct mtk_dpi *dpi)
 		mtk_dpi_mask(dpi, dpi->conf->reg_h_fre_con, 0, EDGE_SEL_EN);
 }
 
+static void mtk_dpi_matrix_sel(struct mtk_dpi *dpi,
+			       enum mtk_dpi_out_color_format format)
+{
+	u32 matrix_sel = 0;
+
+	if (!dpi->conf->color_fmt_trans_support) {
+		dev_info(dpi->dev, "matrix_sel is not supported.\n");
+		return;
+	}
+
+	switch (format) {
+	case MTK_DPI_COLOR_FORMAT_YCBCR_422:
+	case MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL:
+	case MTK_DPI_COLOR_FORMAT_YCBCR_444:
+	case MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL:
+	case MTK_DPI_COLOR_FORMAT_XV_YCC:
+		if (dpi->mode.hdisplay <= 720)
+			matrix_sel = 0x2;
+		break;
+	default:
+		break;
+	}
+	mtk_dpi_mask(dpi, DPI_MATRIX_SET, matrix_sel, INT_MATRIX_SEL_MASK);
+}
+
 static void mtk_dpi_config_color_format(struct mtk_dpi *dpi,
 					enum mtk_dpi_out_color_format format)
 {
@@ -419,6 +446,7 @@  static void mtk_dpi_config_color_format(struct mtk_dpi *dpi,
 	    (format == MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL)) {
 		mtk_dpi_config_yuv422_enable(dpi, false);
 		mtk_dpi_config_csc_enable(dpi, true);
+		mtk_dpi_matrix_sel(dpi, format);
 		if (dpi->conf->swap_input_support)
 			mtk_dpi_config_swap_input(dpi, false);
 		mtk_dpi_config_channel_swap(dpi, MTK_DPI_OUT_CHANNEL_SWAP_BGR);
@@ -426,6 +454,7 @@  static void mtk_dpi_config_color_format(struct mtk_dpi *dpi,
 		   (format == MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL)) {
 		mtk_dpi_config_yuv422_enable(dpi, true);
 		mtk_dpi_config_csc_enable(dpi, true);
+		mtk_dpi_matrix_sel(dpi, format);
 		if (dpi->conf->swap_input_support)
 			mtk_dpi_config_swap_input(dpi, true);
 		else
@@ -673,7 +702,10 @@  static int mtk_dpi_bridge_atomic_check(struct drm_bridge *bridge,
 	dpi->bit_num = MTK_DPI_OUT_BIT_NUM_8BITS;
 	dpi->channel_swap = MTK_DPI_OUT_CHANNEL_SWAP_RGB;
 	dpi->yc_map = MTK_DPI_OUT_YC_MAP_RGB;
-	dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;
+	if (out_bus_format == MEDIA_BUS_FMT_YUYV8_1X16)
+		dpi->color_format = MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL;
+	else
+		dpi->color_format = MTK_DPI_COLOR_FORMAT_RGB;
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
index 3a02fabe1662..cca0dccb84a2 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
+++ b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
@@ -217,4 +217,7 @@ 
 
 #define EDGE_SEL_EN			BIT(5)
 #define H_FRE_2N			BIT(25)
+
+#define DPI_MATRIX_SET		0xB4
+#define INT_MATRIX_SEL_MASK		GENMASK(4, 0)
 #endif /* __MTK_DPI_REGS_H */