mbox series

[v14,00/15] drm/mediatek: Add MT8195 dp_intf driver

Message ID 20220624030946.14961-1-rex-bc.chen@mediatek.com
Headers show
Series drm/mediatek: Add MT8195 dp_intf driver | expand

Message

Rex-BC Chen (陳柏辰) June 24, 2022, 3:09 a.m. UTC
The dpi/dpintf driver and the added helper functions are required for
the DisplayPort driver to work.

This series is separated from [1] which is original from Guillaume.
The display port driver is [2].

Changes for v14:
1. Separate a new binding patch to modify mediatek string format.
2. Use GENMASK(4, 0) for INT_MATRIX_SEL_MASK in patch
   "Add YUV422 output support"
3. Change kernel doc description of support_direct_pin.
4. Change to use pixels_per_iter to control quantity of transferred
   pixels per iterration.

Changes for v13:
1. Change mediatek,mt8195-dp_intf to mediatek,mt8195-dp-intf.
2. Add kernel doc for mtk_dpi_conf.
3. Drop patch of tvd_pll enable.
4. Squash some color format transfer related patches.
5. Add new patch to support setting of direct connection to pins.
6. Change fix tag of "drm/mediatek: dpi: Only enable dpi after the bridge is enabled".

Changes for v12:
1. Remove pll_gate.
2. Add more detailed commit message.
3. Separate tvd_clk patch and yuv422 output support from add dpintf
   support patch
4. Remove limit patch and use common driver codes to determine this.

Changes for v11:
1. Rename ck_cg to pll_gate.
2. Add some commit message to clarify the modification reason.
3. Fix some driver order and modify for reviewers' comments.

[1]:https://lore.kernel.org/all/20220523104758.29531-1-granquet@baylibre.com/
[2]:https://lore.kernel.org/all/20220610105522.13449-1-rex-bc.chen@mediatek.com/

Bo-Chen Chen (5):
  dt-bindings: mediatek,dpi: Revise mediatek strings to correct format
  drm/mediatek: dpi: Add kernel document for struct mtk_dpi_conf
  drm/mediatek: dpi: Add support for quantization range
  drm/mediatek: dpi: Add YUV422 output support
  drm/mediatek: dpi: add config to support direct connection to dpi
    panels

Guillaume Ranquet (9):
  drm/mediatek: dpi: implement a CK/DE pol toggle in SoC config
  drm/mediatek: dpi: implement a swap_input toggle in SoC config
  drm/mediatek: dpi: move dimension mask to SoC config
  drm/mediatek: dpi: move hvsize_mask to SoC config
  drm/mediatek: dpi: move swap_shift to SoC config
  drm/mediatek: dpi: move the yuv422_en_bit to SoC config
  drm/mediatek: dpi: move the csc_enable bit to SoC config
  drm/mediatek: dpi: Add dp_intf support
  drm/mediatek: dpi: Only enable dpi after the bridge is enabled

Markus Schneider-Pargmann (1):
  dt-bindings: mediatek,dpi: Add DP_INTF compatible

 .../display/mediatek/mediatek,dpi.yaml        |  11 +-
 drivers/gpu/drm/mediatek/mtk_dpi.c            | 268 +++++++++++++++---
 drivers/gpu/drm/mediatek/mtk_dpi_regs.h       |  15 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c   |   4 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c        |   3 +
 6 files changed, 252 insertions(+), 50 deletions(-)

Comments

CK Hu (胡俊光) June 27, 2022, 8:54 a.m. UTC | #1
Hi, Bo-Chen:

On Fri, 2022-06-24 at 11:09 +0800, Bo-Chen Chen wrote:
> This driver will support dp_intf and there are many configs between
> dpi
> and dp_intf. Therefore, we will add many configs in "struct
> mtk_dpi_conf".
> To let this structure more readable, we add this kernel doc.
> 

Reviewed-by: CK Hu <ck.hu@mediatek.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 | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
> b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index e61cd67b978f..f66a121ba0c9 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -118,6 +118,15 @@ struct mtk_dpi_yc_limit {
>  	u16 c_bottom;
>  };
>  
> +/**
> + * struct mtk_dpi_conf - Configuration of mediatek dpi.
> + * @cal_factor: Callback function to calculate factor value.
> + * @reg_h_fre_con: Register address of frequency control.
> + * @max_clock_khz: Max clock frequency supported for this SoCs in
> khz units.
> + * @edge_sel_en: Enable of edge selection.
> + * @output_fmts: Array of supported output formats.
> + * @num_output_fmts: Quantity of supported output formats.
> + */
>  struct mtk_dpi_conf {
>  	unsigned int (*cal_factor)(int clock);
>  	u32 reg_h_fre_con;
AngeloGioacchino Del Regno June 27, 2022, 10:52 a.m. UTC | #2
Il 24/06/22 05:09, Bo-Chen Chen ha scritto:
> From: Guillaume Ranquet <granquet@baylibre.com>
> 
> Dpintf is the displayport interface hardware unit. This unit is similar
> to dpi and can reuse most of the code.
> 
> This patch adds support for mt8195-dpintf to this dpi driver. Main
> differences are:
>   - 4 pixels for one iteration for dp_intf while dpi is 1 pixel for one
>     iteration. Therefore, we add a new config "pixels_per_iter" to control
>     quantity of transferred pixels per iteration.
>   - Input of dp_intf is two pixels per iteration, so we add a new config
>     "input_2pixel" to control this.
>   - Some register contents differ slightly between the two components. To
>     work around this I added register bits/masks with a DPINTF_ prefix
>     and use them where different.
> 
> Based on a separate driver for dpintf created by
> Jitao shi <jitao.shi@mediatek.com>.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
> [Bo-Chen: Modify reviewers' comments.]
> Signed-off-by: Bo-Chen Chen <rex-bc.chen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
AngeloGioacchino Del Regno June 27, 2022, 10:52 a.m. UTC | #3
Il 24/06/22 05:09, Bo-Chen Chen ha scritto:
> From: Guillaume Ranquet <granquet@baylibre.com>
> 
> Enabling the dpi too early causes glitches on screen.
> 
> Move the call to mtk_dpi_enable() at the end of the bridge_enable
> callback to ensure everything is setup properly before enabling dpi.
> 
> Fixes: 9e629c17aa8d ("drm/mediatek: Add DPI sub driver")
> Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
> Signed-off-by: Bo-Chen Chen <rex-bc.chen@mediatek.com>

Tested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
CK Hu (胡俊光) June 28, 2022, 2:15 a.m. UTC | #4
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 | #5
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 | #6
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 | #7
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 | #8
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 | #9
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 */
> > > > > 
> > > > > 
> > > > 
> > > > 
> > > 
> > > 
> > 
> > 
> 
>