mbox series

[v4,00/17] Support IGT in display driver

Message ID 20231212121957.19231-1-shawn.sung@mediatek.com
Headers show
Series Support IGT in display driver | expand

Message

Shawn Sung Dec. 12, 2023, 12:19 p.m. UTC
This series is based on mediatek-drm-next.

This series adds support for running IGT (Intel GPU Tool) tests
with MediaTek display driver. The following changes will be
applied:

1. Add a new API for creating GCE thread loop to retrieve CRCs
   from the hardware component
2. Support hardware CRC calculation in both VDOSYS0 and VDOSYS1
3. Support alpha blending in both VDOSYS0 and VDOSYS1

Changes in v4:
- Seperate the patch into smaller ones
- Change the title of some patches
- Revert the changes that are not related to the series

Changes in v3:
- Modify the dt-binding document of Mediatek OVL
- Set DRM mode configs accroding to the hardware capabilities
- Replace cmdq_pkt_jump_absolute() with cmdq_pkt_jump()

Changes in v2:
- Simplify CMDQ by adding commands that are currently used only
- Integrate CRC related codes into new APIs for Mixer and OVL to reuse
- Add CPU version CRC retrieval when CMDQ is disabled

Hsiao Chien Sung (17):
  soc: mediatek: Add register definitions for GCE
  soc: mediatek: Disable 9-bit alpha in ETHDR
  dt-bindings: display: mediatek: ovl: Modify rules for MT8195/MT8188
  drm/mediatek: Add OVL compatible name for MT8195
  drm/mediatek: Set DRM mode configs accordingly
  drm/mediatek: Support alpha blending in OVL
  drm/mediatek: Support alpha blending in Mixer
  drm/mediatek: Support alpha blending in display driver
  drm/mediatek: Support CSC in OVL
  drm/mediatek: Support more color formats in OVL
  drm/mediatek: Turn off the layers with zero width or height
  drm/mediatek: Support CRC in display driver
  drm/mediatek: Support CRC in OVL
  drm/mediatek: Support CRC in OVL adaptor
  drm/mediatek: Add missing plane settings when async update
  drm/mediatek: Fix errors when reporting rotation capability
  drm/mediatek: Add comments for the structures

 .../display/mediatek/mediatek,ovl.yaml        |  12 +-
 drivers/gpu/drm/mediatek/mtk_disp_drv.h       |   7 +
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c       | 326 +++++++++++++++---
 .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c   |  32 +-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c       | 261 +++++++++++++-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.h       |  39 +++
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c   |   7 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  35 ++
 drivers/gpu/drm/mediatek/mtk_drm_drv.c        |  30 +-
 drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  16 +
 drivers/gpu/drm/mediatek/mtk_drm_plane.c      |  15 +-
 drivers/gpu/drm/mediatek/mtk_ethdr.c          | 106 +++++-
 drivers/gpu/drm/mediatek/mtk_ethdr.h          |   5 +
 drivers/soc/mediatek/mtk-mmsys.c              |   1 +
 include/linux/soc/mediatek/mtk-cmdq.h         |  10 +
 15 files changed, 834 insertions(+), 68 deletions(-)

--
2.18.0

Comments

AngeloGioacchino Del Regno Dec. 12, 2023, 1:27 p.m. UTC | #1
Il 12/12/23 13:19, Hsiao Chien Sung ha scritto:
> Add comments for the structures to improve readability.
> 
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>   drivers/gpu/drm/mediatek/mtk_disp_ovl.c     | 21 +++++++++++++-
>   drivers/gpu/drm/mediatek/mtk_drm_crtc.c     | 22 ++++++++++++--
>   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 32 +++++++++++++++++++++
>   drivers/gpu/drm/mediatek/mtk_drm_drv.h      | 15 ++++++++++
>   drivers/gpu/drm/mediatek/mtk_ethdr.c        | 11 +++++++
>   5 files changed, 97 insertions(+), 4 deletions(-)
> 

..snip..

> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index 38d08796fae4..af80c9e50d36 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -46,6 +46,38 @@ enum mtk_ddp_comp_type {
>   
>   struct mtk_ddp_comp;
>   struct cmdq_pkt;
> +
> +/* struct mtk_ddp_comp_funcs - function pointers of the ddp components
> + * @clk_enable: enable the clocks of the component
> + * @clk_disable: disable the clocks of the component
> + * @config: configure the component
> + * @start: start (enable) the component
> + * @stop: stop (disable) the component
> + * @register_vblank_cb: to register a callback function when vblank irq occurs
> + * @unregister_vblank_cb: to unregister the callback function from the vblank irq
> + * @enable_vblank: enable vblank irq
> + * @disable_vblank: disable vblank irq
> + * @supported_rotations: return rotation capability of the component
> + * @layer_nr: how many layers the component supports
> + * @layer_check: to check if the state of the layer is valid for the component
> + * @layer_config: to configure the component according to the state of the layer
> + * @gamma_set: to set gamma for the component
> + * @bgclr_in_on: turn on background color
> + * @bgclr_in_off: turn off background color
> + * @ctm_set: set color transformation matrix
> + * @dma_dev_get: return the device that uses direct memory access
> + * @get_formats: get the format that is currently in use by the component
> + * @get_num_formats: get number of the formats that the component supports
> + * @connect: connect the sub modules of the component
> + * @disconnect: disconnect the sub modules of the component
> + * @add: add the device to the component (mount them in the mutex)
> + * @remove: remove the device from the component (unmount them from the mutex)
> + * @encoder_index: get the encoder index of the component
> + * @crc: return the start of crc array
> + * @crc_cnt: how many CRCs the component supports
> + * @crc_entry: get the pointer to the crc entry
> + * @crc_read: call this function to read crc from the hardware component
> + */
>   struct mtk_ddp_comp_funcs {
>   	int (*power_on)(struct device *dev);
>   	void (*power_off)(struct device *dev);

Please rebase over the latest upstream kernel, as it doesn't apply like this.

After which:
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
AngeloGioacchino Del Regno Dec. 12, 2023, 1:27 p.m. UTC | #2
Il 12/12/23 13:19, Hsiao Chien Sung ha scritto:
> We found that IGT (Intel GPU Tool) will try to commit layers with
> zero width or height and lead to undefined behaviors in hardware.
> Disable the layers in such situations.
> 
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>

This commit needs a Fixes tag. Please add the relevant one.

Thanks,
Angelo

> ---
>   drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 2 +-
>   drivers/gpu/drm/mediatek/mtk_ethdr.c            | 7 ++++++-
>   2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> index 10d23e76acaa..8789442c039f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> @@ -156,7 +156,7 @@ void mtk_ovl_adaptor_layer_config(struct device *dev, unsigned int idx,
>   	merge = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_MERGE0 + idx];
>   	ethdr = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0];
>   
> -	if (!pending->enable) {
> +	if (!pending->enable || !pending->width || !pending->height) {
>   		mtk_merge_stop_cmdq(merge, cmdq_pkt);
>   		mtk_mdp_rdma_stop(rdma_l, cmdq_pkt);
>   		mtk_mdp_rdma_stop(rdma_r, cmdq_pkt);
> diff --git a/drivers/gpu/drm/mediatek/mtk_ethdr.c b/drivers/gpu/drm/mediatek/mtk_ethdr.c
> index 73c9e3da56a7..e95331c06815 100644
> --- a/drivers/gpu/drm/mediatek/mtk_ethdr.c
> +++ b/drivers/gpu/drm/mediatek/mtk_ethdr.c
> @@ -163,7 +163,12 @@ void mtk_ethdr_layer_config(struct device *dev, unsigned int idx,
>   	if (idx >= 4)
>   		return;
>   
> -	if (!pending->enable) {
> +	if (!pending->enable || !pending->width || !pending->height) {
> +		/*
> +		 * instead of disabling layer with MIX_SRC_CON directly
> +		 * set the size to 0 to avoid screen shift due to mixer
> +		 * mode switch (hardware behavior)
> +		 */
>   		mtk_ddp_write(cmdq_pkt, 0, &mixer->cmdq_base, mixer->regs, MIX_L_SRC_SIZE(idx));
>   		return;
>   	}
AngeloGioacchino Del Regno Dec. 12, 2023, 1:27 p.m. UTC | #3
Il 12/12/23 13:19, Hsiao Chien Sung ha scritto:
> Set DRM mode configs limitation accroding to the
> hardware capabilities.
> 
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>   drivers/gpu/drm/mediatek/mtk_drm_drv.c | 28 ++++++++++++++++++--------
>   drivers/gpu/drm/mediatek/mtk_drm_drv.h |  1 +
>   2 files changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 5d551bff6b3f..a4b740420ebb 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -304,6 +304,7 @@ static const struct mtk_mmsys_driver_data mt8188_vdosys0_driver_data = {
>   	.conn_routes = mt8188_mtk_ddp_main_routes,
>   	.conn_routes_num = ARRAY_SIZE(mt8188_mtk_ddp_main_routes),
>   	.mmsys_dev_num = 2,
> +	.max_pitch = GENMASK(15, 0),
>   };
>   
>   static const struct mtk_mmsys_driver_data mt8192_mmsys_driver_data = {
> @@ -318,6 +319,7 @@ static const struct mtk_mmsys_driver_data mt8195_vdosys0_driver_data = {
>   	.main_path = mt8195_mtk_ddp_main,
>   	.main_len = ARRAY_SIZE(mt8195_mtk_ddp_main),
>   	.mmsys_dev_num = 2,
> +	.max_pitch = GENMASK(15, 0),
>   };
>   
>   static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data = {
> @@ -325,6 +327,7 @@ static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data = {
>   	.ext_len = ARRAY_SIZE(mt8195_mtk_ddp_ext),
>   	.mmsys_id = 1,
>   	.mmsys_dev_num = 2,
> +	.max_pitch = GENMASK(15, 0),
>   };
>   
>   static const struct of_device_id mtk_drm_of_ids[] = {
> @@ -463,16 +466,16 @@ static int mtk_drm_kms_init(struct drm_device *drm)
>   	if (ret)
>   		goto put_mutex_dev;
>   
> -	drm->mode_config.min_width = 64;
> -	drm->mode_config.min_height = 64;
> -
>   	/*
> -	 * set max width and height as default value(4096x4096).
> -	 * this value would be used to check framebuffer size limitation
> -	 * at drm_mode_addfb().
> +	 * Set default values for drm mode config
> +	 * these values will be referenced by drm_mode_addfb() as
> +	 * frame buffer size limitation.
>   	 */
> -	drm->mode_config.max_width = 4096;
> -	drm->mode_config.max_height = 4096;
> +	drm->mode_config.min_width = 1;
> +	drm->mode_config.min_height = 1;
> +	drm->mode_config.cursor_width = 512;
> +	drm->mode_config.cursor_height = 512;
> +
>   	drm->mode_config.funcs = &mtk_drm_mode_config_funcs;
>   	drm->mode_config.helper_private = &mtk_drm_mode_config_helpers;
>   
> @@ -502,6 +505,15 @@ static int mtk_drm_kms_init(struct drm_device *drm)
>   		for (j = 0; j < private->data->mmsys_dev_num; j++) {
>   			priv_n = private->all_drm_private[j];
>   
> +			if (priv_n->data->max_pitch) {
> +				/* Save 4 bytes for the color depth (pitch = width x bpp) */

This comment is confusing. Did you mean 4 *bits*? Four bytes is 32 bits.

Also, I'd change the last part to "(pitch = [ width or height ] x bpp)"

> +				drm->mode_config.max_width  = priv_n->data->max_pitch >> 2;
> +				drm->mode_config.max_height = priv_n->data->max_pitch >> 2;
> +			} else {
> +				drm->mode_config.max_width = 4096;
> +				drm->mode_config.max_height = 4096;
> +			}
> +
>   			if (i == 0 && priv_n->data->main_len) {
>   				ret = mtk_drm_crtc_create(drm, priv_n->data->main_path,
>   							  priv_n->data->main_len, j,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> index d2efd715699f..3d6c1f58a7ec 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> @@ -41,6 +41,7 @@ struct mtk_mmsys_driver_data {
>   	bool shadow_register;
>   	unsigned int mmsys_id;
>   	unsigned int mmsys_dev_num;
> +	u32 max_pitch;

Is it expected to have a max_pitch > 0xFFFF on newer SoCs?
If not, please change this to u16.

Regards,
Angelo
AngeloGioacchino Del Regno Dec. 12, 2023, 1:27 p.m. UTC | #4
Il 12/12/23 13:19, Hsiao Chien Sung ha scritto:
> Add OVL compatible name for MT8195.
> 
> Without this commit, DRM won't work after modifying
> the device tree.
> 
> Reviewed-by: CK Hu <ck.hu@mediatek.com>
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
CK Hu (胡俊光) Jan. 2, 2024, 3:36 a.m. UTC | #5
Hi, Hsiao-chien:

On Tue, 2023-12-12 at 20:19 +0800, Hsiao Chien Sung wrote:
> Set DRM mode configs limitation accroding to the
> hardware capabilities.
> 
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 28 ++++++++++++++++++----
> ----
>  drivers/gpu/drm/mediatek/mtk_drm_drv.h |  1 +
>  2 files changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 5d551bff6b3f..a4b740420ebb 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -304,6 +304,7 @@ static const struct mtk_mmsys_driver_data
> mt8188_vdosys0_driver_data = {
>  	.conn_routes = mt8188_mtk_ddp_main_routes,
>  	.conn_routes_num = ARRAY_SIZE(mt8188_mtk_ddp_main_routes),
>  	.mmsys_dev_num = 2,
> +	.max_pitch = GENMASK(15, 0),
>  };
>  
>  static const struct mtk_mmsys_driver_data mt8192_mmsys_driver_data =
> {
> @@ -318,6 +319,7 @@ static const struct mtk_mmsys_driver_data
> mt8195_vdosys0_driver_data = {
>  	.main_path = mt8195_mtk_ddp_main,
>  	.main_len = ARRAY_SIZE(mt8195_mtk_ddp_main),
>  	.mmsys_dev_num = 2,
> +	.max_pitch = GENMASK(15, 0),
>  };
>  
>  static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data
> = {
> @@ -325,6 +327,7 @@ static const struct mtk_mmsys_driver_data
> mt8195_vdosys1_driver_data = {
>  	.ext_len = ARRAY_SIZE(mt8195_mtk_ddp_ext),
>  	.mmsys_id = 1,
>  	.mmsys_dev_num = 2,
> +	.max_pitch = GENMASK(15, 0),
>  };
>  
>  static const struct of_device_id mtk_drm_of_ids[] = {
> @@ -463,16 +466,16 @@ static int mtk_drm_kms_init(struct drm_device
> *drm)
>  	if (ret)
>  		goto put_mutex_dev;
>  
> -	drm->mode_config.min_width = 64;
> -	drm->mode_config.min_height = 64;
> -
>  	/*
> -	 * set max width and height as default value(4096x4096).
> -	 * this value would be used to check framebuffer size
> limitation
> -	 * at drm_mode_addfb().
> +	 * Set default values for drm mode config
> +	 * these values will be referenced by drm_mode_addfb() as
> +	 * frame buffer size limitation.
>  	 */
> -	drm->mode_config.max_width = 4096;
> -	drm->mode_config.max_height = 4096;
> +	drm->mode_config.min_width = 1;
> +	drm->mode_config.min_height = 1;
> +	drm->mode_config.cursor_width = 512;
> +	drm->mode_config.cursor_height = 512;

Why do you change other SoC's min_width/min_height and
cursor_width/cursor_height? Describe the reason.

Regards,
CK

> +
>  	drm->mode_config.funcs = &mtk_drm_mode_config_funcs;
>  	drm->mode_config.helper_private = &mtk_drm_mode_config_helpers;
>  
> @@ -502,6 +505,15 @@ static int mtk_drm_kms_init(struct drm_device
> *drm)
>  		for (j = 0; j < private->data->mmsys_dev_num; j++) {
>  			priv_n = private->all_drm_private[j];
>  
> +			if (priv_n->data->max_pitch) {
> +				/* Save 4 bytes for the color depth
> (pitch = width x bpp) */
> +				drm->mode_config.max_width  = priv_n-
> >data->max_pitch >> 2;
> +				drm->mode_config.max_height = priv_n-
> >data->max_pitch >> 2;
> +			} else {
> +				drm->mode_config.max_width = 4096;
> +				drm->mode_config.max_height = 4096;
> +			}
> +
>  			if (i == 0 && priv_n->data->main_len) {
>  				ret = mtk_drm_crtc_create(drm, priv_n-
> >data->main_path,
>  							  priv_n->data-
> >main_len, j,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> index d2efd715699f..3d6c1f58a7ec 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> @@ -41,6 +41,7 @@ struct mtk_mmsys_driver_data {
>  	bool shadow_register;
>  	unsigned int mmsys_id;
>  	unsigned int mmsys_dev_num;
> +	u32 max_pitch;
>  };
>  
>  struct mtk_drm_private {
CK Hu (胡俊光) Jan. 2, 2024, 5:43 a.m. UTC | #6
Hi, Hsiao-chien:

On Tue, 2023-12-12 at 20:19 +0800, Hsiao Chien Sung wrote:
> Support premultiply and coverage alpha blending in
> Overlay.

Describe what kind of alpha blending already support for cursor plane.
And separate premultiply alpha and coverage alpha to two patches.

> 
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 73 +++++++++++++++++----
> ----
>  1 file changed, 51 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 5aaf4342cdbd..66074c2d917c 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -39,6 +39,7 @@
>  #define DISP_REG_OVL_PITCH_MSB(n)		(0x0040 + 0x20 * (n))
>  #define OVL_PITCH_MSB_2ND_SUBBUF			BIT(16)
>  #define DISP_REG_OVL_PITCH(n)			(0x0044 + 0x20
> * (n))
> +#define OVL_CONST_BLEND					BIT(28)
>  #define DISP_REG_OVL_RDMA_CTRL(n)		(0x00c0 + 0x20 * (n))
>  #define DISP_REG_OVL_RDMA_GMC(n)		(0x00c8 + 0x20 * (n))
>  #define DISP_REG_OVL_ADDR_MT2701		0x0040
> @@ -52,13 +53,16 @@
>  #define GMC_THRESHOLD_HIGH	((1 << GMC_THRESHOLD_BITS) / 4)
>  #define GMC_THRESHOLD_LOW	((1 << GMC_THRESHOLD_BITS) / 8)
>  
> -#define OVL_CON_BYTE_SWAP	BIT(24)
> -#define OVL_CON_MTX_YUV_TO_RGB	(6 << 16)
> -#define OVL_CON_CLRFMT_RGB	(1 << 12)
> -#define OVL_CON_CLRFMT_RGBA8888	(2 << 12)
> -#define OVL_CON_CLRFMT_ARGB8888	(3 << 12)
> -#define OVL_CON_CLRFMT_UYVY	(4 << 12)
> -#define OVL_CON_CLRFMT_YUYV	(5 << 12)
> +#define OVL_CON_CLRFMT_MAN		BIT(23)
> +#define OVL_CON_BYTE_SWAP		BIT(24)
> +#define OVL_CON_RGB_SWAP		BIT(25)
> +#define OVL_CON_CLRFMT_RGB		(1 << 12)
> +#define OVL_CON_CLRFMT_RGBA8888		(2 << 12)
> +#define OVL_CON_CLRFMT_ARGB8888		(3 << 12)
> +#define OVL_CON_CLRFMT_PARGB8888	(OVL_CON_CLRFMT_ARGB8888 |
> OVL_CON_CLRFMT_MAN)
> +#define OVL_CON_CLRFMT_UYVY		(4 << 12)
> +#define OVL_CON_CLRFMT_YUYV		(5 << 12)
> +#define OVL_CON_MTX_YUV_TO_RGB		(6 << 16)
>  #define OVL_CON_CLRFMT_RGB565(ovl)	((ovl)->data->fmt_rgb565_is_0 ?
> \
>  					0 : OVL_CON_CLRFMT_RGB)
>  #define OVL_CON_CLRFMT_RGB888(ovl)	((ovl)->data->fmt_rgb565_is_0 ?
> \
> @@ -208,14 +212,12 @@ void mtk_ovl_clk_disable(struct device *dev)
>  void mtk_ovl_start(struct device *dev)
>  {
>  	struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
> +	unsigned int reg = readl(ovl->regs +
> DISP_REG_OVL_DATAPATH_CON);
>  
> -	if (ovl->data->smi_id_en) {
> -		unsigned int reg;
> +	if (ovl->data->smi_id_en)
> +		reg |= OVL_LAYER_SMI_ID_EN;
>  
> -		reg = readl(ovl->regs + DISP_REG_OVL_DATAPATH_CON);
> -		reg = reg | OVL_LAYER_SMI_ID_EN;
> -		writel_relaxed(reg, ovl->regs +
> DISP_REG_OVL_DATAPATH_CON);
> -	}
> +	writel_relaxed(reg, ovl->regs + DISP_REG_OVL_DATAPATH_CON);

Nothing change in this function, so drop this modification.

>  	writel_relaxed(0x1, ovl->regs + DISP_REG_OVL_EN);
>  }
>  
> @@ -274,7 +276,13 @@ void mtk_ovl_config(struct device *dev, unsigned
> int w,
>  	if (w != 0 && h != 0)
>  		mtk_ddp_write_relaxed(cmdq_pkt, h << 16 | w, &ovl-
> >cmdq_reg, ovl->regs,
>  				      DISP_REG_OVL_ROI_SIZE);
> -	mtk_ddp_write_relaxed(cmdq_pkt, 0x0, &ovl->cmdq_reg, ovl->regs, 
> DISP_REG_OVL_ROI_BGCLR);
> +
> +	/*
> +	 * The background color should be opaque black (ARGB),
> +	 * otherwise there will be no effect with alpha blend
> +	 */
> +	mtk_ddp_write_relaxed(cmdq_pkt, 0xff000000, &ovl->cmdq_reg,
> +			      ovl->regs, DISP_REG_OVL_ROI_BGCLR);

If cursor plane also has this problem, separate this to a bug-fix
patch. If only new alpha blending mode has this problem, describe more
detail why new blending mode has this problem.

Regards,
CK

>  
>  	mtk_ddp_write(cmdq_pkt, 0x1, &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_RST);
>  	mtk_ddp_write(cmdq_pkt, 0x0, &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_RST);
> @@ -357,7 +365,8 @@ void mtk_ovl_layer_off(struct device *dev,
> unsigned int idx,
>  		      DISP_REG_OVL_RDMA_CTRL(idx));
>  }
>  
> -static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl,
> unsigned int fmt)
> +static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl,
> unsigned int fmt,
> +				    unsigned int blend_mode)
>  {
>  	/* The return value in switch "MEM_MODE_INPUT_FORMAT_XXX"
>  	 * is defined in mediatek HW data sheet.
> @@ -376,18 +385,30 @@ static unsigned int ovl_fmt_convert(struct
> mtk_disp_ovl *ovl, unsigned int fmt)
>  		return OVL_CON_CLRFMT_RGB888(ovl) | OVL_CON_BYTE_SWAP;
>  	case DRM_FORMAT_RGBX8888:
>  	case DRM_FORMAT_RGBA8888:
> -		return OVL_CON_CLRFMT_ARGB8888;
> +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> +		       OVL_CON_CLRFMT_ARGB8888 :
> +		       OVL_CON_CLRFMT_PARGB8888;
>  	case DRM_FORMAT_BGRX8888:
>  	case DRM_FORMAT_BGRA8888:
> +		return OVL_CON_BYTE_SWAP |
> +		       (blend_mode == DRM_MODE_BLEND_COVERAGE ?
> +		       OVL_CON_CLRFMT_ARGB8888 :
> +		       OVL_CON_CLRFMT_PARGB8888);
>  	case DRM_FORMAT_BGRA1010102:
>  		return OVL_CON_CLRFMT_ARGB8888 | OVL_CON_BYTE_SWAP;
>  	case DRM_FORMAT_XRGB8888:
>  	case DRM_FORMAT_ARGB8888:
> +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> +		       OVL_CON_CLRFMT_RGBA8888 :
> +		       OVL_CON_CLRFMT_PARGB8888;
>  	case DRM_FORMAT_ARGB2101010:
>  		return OVL_CON_CLRFMT_RGBA8888;
>  	case DRM_FORMAT_XBGR8888:
>  	case DRM_FORMAT_ABGR8888:
> -		return OVL_CON_CLRFMT_RGBA8888 | OVL_CON_BYTE_SWAP;
> +		return OVL_CON_RGB_SWAP |
> +		       (blend_mode == DRM_MODE_BLEND_COVERAGE ?
> +		       OVL_CON_CLRFMT_RGBA8888 :
> +		       OVL_CON_CLRFMT_PARGB8888);
>  	case DRM_FORMAT_UYVY:
>  		return OVL_CON_CLRFMT_UYVY | OVL_CON_MTX_YUV_TO_RGB;
>  	case DRM_FORMAT_YUYV:
> @@ -408,6 +429,8 @@ void mtk_ovl_layer_config(struct device *dev,
> unsigned int idx,
>  	unsigned int fmt = pending->format;
>  	unsigned int offset = (pending->y << 16) | pending->x;
>  	unsigned int src_size = (pending->height << 16) | pending-
> >width;
> +	unsigned int blend_mode = state->base.pixel_blend_mode;
> +	unsigned int ignore_pixel_alpha = 0;
>  	unsigned int con;
>  	bool is_afbc = pending->modifier != DRM_FORMAT_MOD_LINEAR;
>  	union overlay_pitch {
> @@ -425,9 +448,15 @@ void mtk_ovl_layer_config(struct device *dev,
> unsigned int idx,
>  		return;
>  	}
>  
> -	con = ovl_fmt_convert(ovl, fmt);
> -	if (state->base.fb && state->base.fb->format->has_alpha)
> -		con |= OVL_CON_AEN | OVL_CON_ALPHA;
> +	con = ovl_fmt_convert(ovl, fmt, blend_mode);
> +	if (state->base.fb) {
> +		con |= OVL_CON_AEN;
> +		con |= state->base.alpha & OVL_CON_ALPHA;
> +	}
> +
> +	if (blend_mode == DRM_MODE_BLEND_PIXEL_NONE ||
> +	    (state->base.fb && !state->base.fb->format->has_alpha))
> +		ignore_pixel_alpha = OVL_CONST_BLEND;
>  
>  	if (pending->rotation & DRM_MODE_REFLECT_Y) {
>  		con |= OVL_CON_VIRT_FLIP;
> @@ -444,8 +473,8 @@ void mtk_ovl_layer_config(struct device *dev,
> unsigned int idx,
>  
>  	mtk_ddp_write_relaxed(cmdq_pkt, con, &ovl->cmdq_reg, ovl->regs,
>  			      DISP_REG_OVL_CON(idx));
> -	mtk_ddp_write_relaxed(cmdq_pkt, overlay_pitch.split_pitch.lsb,
> &ovl->cmdq_reg, ovl->regs,
> -			      DISP_REG_OVL_PITCH(idx));
> +	mtk_ddp_write_relaxed(cmdq_pkt, overlay_pitch.split_pitch.lsb |
> ignore_pixel_alpha,
> +			      &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_PITCH(idx));
>  	mtk_ddp_write_relaxed(cmdq_pkt, src_size, &ovl->cmdq_reg, ovl-
> >regs,
>  			      DISP_REG_OVL_SRC_SIZE(idx));
>  	mtk_ddp_write_relaxed(cmdq_pkt, offset, &ovl->cmdq_reg, ovl-
> >regs,
CK Hu (胡俊光) Jan. 2, 2024, 5:54 a.m. UTC | #7
Hi, Hsiao-chien:

On Tue, 2023-12-12 at 20:19 +0800, Hsiao Chien Sung wrote:
> Support premultiply and coverage alpha blending in
> Mixer.

Describe what kind of alpha blending already support for cursor plane.
And separate premultiply alpha and coverage alpha to two patches.

Regards,
CK

> 
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_ethdr.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_ethdr.c
> b/drivers/gpu/drm/mediatek/mtk_ethdr.c
> index 73dc4da3ba3b..73c9e3da56a7 100644
> --- a/drivers/gpu/drm/mediatek/mtk_ethdr.c
> +++ b/drivers/gpu/drm/mediatek/mtk_ethdr.c
> @@ -5,6 +5,7 @@
>  
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_framebuffer.h>
> +#include <drm/drm_blend.h>
>  #include <linux/clk.h>
>  #include <linux/component.h>
>  #include <linux/of_device.h>
> @@ -35,6 +36,7 @@
>  #define MIX_SRC_L0_EN				BIT(0)
>  #define MIX_L_SRC_CON(n)		(0x28 + 0x18 * (n))
>  #define NON_PREMULTI_SOURCE			(2 << 12)
> +#define PREMULTI_SOURCE				(3 << 12)
>  #define MIX_L_SRC_SIZE(n)		(0x30 + 0x18 * (n))
>  #define MIX_L_SRC_OFFSET(n)		(0x34 + 0x18 * (n))
>  #define MIX_FUNC_DCM0			0x120
> @@ -153,7 +155,8 @@ void mtk_ethdr_layer_config(struct device *dev,
> unsigned int idx,
>  	struct mtk_plane_pending_state *pending = &state->pending;
>  	unsigned int offset = (pending->x & 1) << 31 | pending->y << 16
> | pending->x;
>  	unsigned int align_width = ALIGN_DOWN(pending->width, 2);
> -	unsigned int alpha_con = 0;
> +	unsigned int mix_con = NON_PREMULTI_SOURCE;
> +	bool replace_src_a = false;
>  
>  	dev_dbg(dev, "%s+ idx:%d", __func__, idx);
>  
> @@ -165,19 +168,28 @@ void mtk_ethdr_layer_config(struct device *dev,
> unsigned int idx,
>  		return;
>  	}
>  
> -	if (state->base.fb && state->base.fb->format->has_alpha)
> -		alpha_con = MIXER_ALPHA_AEN | MIXER_ALPHA;
> +	mix_con |= MIXER_ALPHA_AEN | (state->base.alpha & MIXER_ALPHA);
>  
> -	mtk_mmsys_mixer_in_config(priv->mmsys_dev, idx + 1, alpha_con ?
> false : true,
> -				  DEFAULT_9BIT_ALPHA,
> +	if (state->base.pixel_blend_mode != DRM_MODE_BLEND_COVERAGE)
> +		mix_con |= PREMULTI_SOURCE;
> +
> +	if (state->base.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE
> ||
> +	    (state->base.fb && !state->base.fb->format->has_alpha)) {
> +		/*
> +		 * Mixer doesn't support CONST_BLD mode,
> +		 * use a trick to make the output equivalent
> +		 */
> +		replace_src_a = true;
> +	}
> +
> +	mtk_mmsys_mixer_in_config(priv->mmsys_dev, idx + 1,
> replace_src_a, MIXER_ALPHA,
>  				  pending->x & 1 ?
> MIXER_INX_MODE_EVEN_EXTEND :
>  				  MIXER_INX_MODE_BYPASS, align_width /
> 2 - 1, cmdq_pkt);
>  
>  	mtk_ddp_write(cmdq_pkt, pending->height << 16 | align_width,
> &mixer->cmdq_base,
>  		      mixer->regs, MIX_L_SRC_SIZE(idx));
>  	mtk_ddp_write(cmdq_pkt, offset, &mixer->cmdq_base, mixer->regs, 
> MIX_L_SRC_OFFSET(idx));
> -	mtk_ddp_write_mask(cmdq_pkt, alpha_con, &mixer->cmdq_base,
> mixer->regs, MIX_L_SRC_CON(idx),
> -			   0x1ff);
> +	mtk_ddp_write(cmdq_pkt, mix_con, &mixer->cmdq_base, mixer-
> >regs, MIX_L_SRC_CON(idx));
>  	mtk_ddp_write_mask(cmdq_pkt, BIT(idx), &mixer->cmdq_base,
> mixer->regs, MIX_SRC_CON,
>  			   BIT(idx));
>  }
CK Hu (胡俊光) Jan. 2, 2024, 6:13 a.m. UTC | #8
Hi, Hsiao-chien:

On Tue, 2023-12-12 at 20:19 +0800, Hsiao Chien Sung wrote:
> Support alpha blending by adding correct blend mode and
> alpha property in plane initialization.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> index 9208f03b3f8c..dfd81172a940 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -349,6 +349,17 @@ int mtk_plane_init(struct drm_device *dev,
> struct drm_plane *plane,
>  			DRM_INFO("Create rotation property failed\n");
>  	}
>  
> +	err = drm_plane_create_alpha_property(plane);
> +	if (err)
> +		DRM_ERROR("failed to create property: alpha\n");
> +
> +	err = drm_plane_create_blend_mode_property(plane,
> +						   BIT(DRM_MODE_BLEND_P
> REMULTI) |
> +						   BIT(DRM_MODE_BLEND_C
> OVERAGE) |
> +						   BIT(DRM_MODE_BLEND_P
> IXEL_NONE));
> +	if (err)
> +		DRM_ERROR("failed to create property: blend_mode\n");
> +
>  	drm_plane_helper_add(plane, &mtk_plane_helper_funcs);
>  
>  	return 0;
CK Hu (胡俊光) Jan. 2, 2024, 7:14 a.m. UTC | #9
Hi, Hsiao-chien:

On Tue, 2023-12-12 at 20:19 +0800, Hsiao Chien Sung wrote:
> Support Color Transform Control (CSC) in Overlay to
> do Y2R or R2R conversion.
> 
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 121
> +++++++++++++++++++++++-
>  1 file changed, 118 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 66074c2d917c..7e217142d0c4 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -31,6 +31,7 @@
>  #define OVL_LAYER_SMI_ID_EN				BIT(0)
>  #define OVL_BGCLR_SEL_IN				BIT(2)
>  #define OVL_LAYER_AFBC_EN(n)				BIT(4+n)
> +#define OVL_OUTPUT_CLAMP				BIT(26)
>  #define DISP_REG_OVL_ROI_BGCLR			0x0028
>  #define DISP_REG_OVL_SRC_CON			0x002c
>  #define DISP_REG_OVL_CON(n)			(0x0030 + 0x20 * (n))
> @@ -44,6 +45,23 @@
>  #define DISP_REG_OVL_RDMA_GMC(n)		(0x00c8 + 0x20 * (n))
>  #define DISP_REG_OVL_ADDR_MT2701		0x0040
>  #define DISP_REG_OVL_CLRFMT_EXT			0x02D0
> +#define DISP_REG_OVL_CLRFMT_EXT1		0x02D8
> +#define OVL_CLRFMT_EXT1_CSC_EN(n)			(1 << (((n) *
> 4) + 1))
> +#define DISP_REG_OVL_Y2R_PARA_R0(n)		(0x0134 + 0x28 * (n))
> +#define OVL_Y2R_PARA_C_CF_RMY				(GENMAS
> K(14, 0))
> +#define DISP_REG_OVL_Y2R_PARA_G0(n)		(0x013c + 0x28 * (n))
> +#define OVL_Y2R_PARA_C_CF_GMU				(GENMAS
> K(30, 16))
> +#define DISP_REG_OVL_Y2R_PARA_B1(n)		(0x0148 + 0x28 * (n))
> +#define OVL_Y2R_PARA_C_CF_BMV				(GENMAS
> K(14, 0))
> +#define DISP_REG_OVL_Y2R_PARA_YUV_A_0(n)	(0x014c + 0x28 * (n))
> +#define OVL_Y2R_PARA_C_CF_YA				(GENMASK(10,
> 0))
> +#define OVL_Y2R_PARA_C_CF_UA				(GENMASK(26,
> 16))
> +#define DISP_REG_OVL_Y2R_PARA_YUV_A_1(n)	(0x0150 + 0x28 * (n))
> +#define OVL_Y2R_PARA_C_CF_VA				(GENMASK(10,
> 0))
> +#define DISP_REG_OVL_Y2R_PRE_ADD2(n)		(0x0154 + 0x28 * (n))
> +#define DISP_REG_OVL_R2R_R0(n)			(0x0500 + 0x40
> * (n))
> +#define DISP_REG_OVL_R2R_G1(n)			(0x0510 + 0x40
> * (n))
> +#define DISP_REG_OVL_R2R_B2(n)			(0x0520 + 0x40
> * (n))
>  #define DISP_REG_OVL_ADDR_MT8173		0x0f40
>  #define DISP_REG_OVL_ADDR(ovl, n)		((ovl)->data->addr +
> 0x20 * (n))
>  #define DISP_REG_OVL_HDR_ADDR(ovl, n)		((ovl)->data-
> >addr + 0x20 * (n) + 0x04)
> @@ -56,6 +74,8 @@
>  #define OVL_CON_CLRFMT_MAN		BIT(23)
>  #define OVL_CON_BYTE_SWAP		BIT(24)
>  #define OVL_CON_RGB_SWAP		BIT(25)
> +#define OVL_CON_MTX_AUTO_DIS		BIT(26)
> +#define OVL_CON_MTX_EN			BIT(27)
>  #define OVL_CON_CLRFMT_RGB		(1 << 12)
>  #define OVL_CON_CLRFMT_RGBA8888		(2 << 12)
>  #define OVL_CON_CLRFMT_ARGB8888		(3 << 12)
> @@ -63,6 +83,7 @@
>  #define OVL_CON_CLRFMT_UYVY		(4 << 12)
>  #define OVL_CON_CLRFMT_YUYV		(5 << 12)
>  #define OVL_CON_MTX_YUV_TO_RGB		(6 << 16)
> +#define OVL_CON_MTX_PROGRAMMABLE	(8 << 16)
>  #define OVL_CON_CLRFMT_RGB565(ovl)	((ovl)->data->fmt_rgb565_is_0 ?
> \
>  					0 : OVL_CON_CLRFMT_RGB)
>  #define OVL_CON_CLRFMT_RGB888(ovl)	((ovl)->data->fmt_rgb565_is_0 ?
> \
> @@ -76,6 +97,22 @@
>  #define	OVL_CON_VIRT_FLIP	BIT(9)
>  #define	OVL_CON_HORZ_FLIP	BIT(10)
>  
> +static inline bool is_10bit_rgb(u32 fmt)
> +{
> +	switch (fmt) {
> +	case DRM_FORMAT_XRGB2101010:
> +	case DRM_FORMAT_ARGB2101010:
> +	case DRM_FORMAT_RGBX1010102:
> +	case DRM_FORMAT_RGBA1010102:
> +	case DRM_FORMAT_XBGR2101010:
> +	case DRM_FORMAT_ABGR2101010:
> +	case DRM_FORMAT_BGRX1010102:
> +	case DRM_FORMAT_BGRA1010102:
> +		return true;
> +	}
> +	return false;
> +}
> +
>  static const u32 mt8173_formats[] = {
>  	DRM_FORMAT_XRGB8888,
>  	DRM_FORMAT_ARGB8888,
> @@ -217,6 +254,14 @@ void mtk_ovl_start(struct device *dev)
>  	if (ovl->data->smi_id_en)
>  		reg |= OVL_LAYER_SMI_ID_EN;
>  
> +	/*
> +	 * When doing Y2R conversion, it's common to get an output
> +	 * that is larger than 10 bits (negative numbers).
> +	 * Enable this bit to clamp the output to 10 bits per channel
> +	 * (should always be enabled)
> +	 */
> +	reg |= OVL_OUTPUT_CLAMP;
> +
>  	writel_relaxed(reg, ovl->regs + DISP_REG_OVL_DATAPATH_CON);
>  	writel_relaxed(0x1, ovl->regs + DISP_REG_OVL_EN);
>  }
> @@ -256,9 +301,7 @@ static void mtk_ovl_set_bit_depth(struct device
> *dev, int idx, u32 format,
>  	reg = readl(ovl->regs + DISP_REG_OVL_CLRFMT_EXT);
>  	reg &= ~OVL_CON_CLRFMT_BIT_DEPTH_MASK(idx);
>  
> -	if (format == DRM_FORMAT_RGBA1010102 ||
> -	    format == DRM_FORMAT_BGRA1010102 ||
> -	    format == DRM_FORMAT_ARGB2101010)
> +	if (is_10bit_rgb(format))
>  		bit_depth = OVL_CON_CLRFMT_10_BIT;
>  
>  	reg |= OVL_CON_CLRFMT_BIT_DEPTH(bit_depth, idx);
> @@ -458,6 +501,78 @@ void mtk_ovl_layer_config(struct device *dev,
> unsigned int idx,
>  	    (state->base.fb && !state->base.fb->format->has_alpha))
>  		ignore_pixel_alpha = OVL_CONST_BLEND;
>  
> +	/* need to do Y2R and R2R to reduce 10bit data to 8bit for CRC
> calculation */

The comment is too simple to understand. I think now do not support 10
bits YUV format, so why Y2R? Describe the mixed result format first. Is
mixed result 8 bit RGB for mt8195? So the problem happen when 10 bit
yuv mixed into 8 bit RGB? I just guess what happen. So describe more
detail about this.

Regards,
CK

> +	if (ovl->data->supports_clrfmt_ext) {
> +		u32 y2r_coef = 0, y2r_offset = 0, r2r_coef = 0, csc_en
> = 0;
> +
> +		if (is_10bit_rgb(fmt)) {
> +			con |= OVL_CON_MTX_AUTO_DIS | OVL_CON_MTX_EN |
> OVL_CON_MTX_PROGRAMMABLE;
> +
> +			/*
> +			 * Y2R coef setting
> +			 * bit 13 is 2^1, bit 12 is 2^0, bit 11 is 2^-
> 1,
> +			 * bit 10 is 2^-2 = 0.25
> +			 */
> +			y2r_coef = BIT(10);
> +
> +			/* -1 in 10bit */
> +			y2r_offset = GENMASK(10, 0) - 1;
> +
> +			/*
> +			 * R2R coef setting
> +			 * bit 19 is 2^1, bit 18 is 2^0, bit 17 is 2^-
> 1,
> +			 * bit 20 is 2^2 = 4
> +			 */
> +			r2r_coef = BIT(20);
> +
> +			/* CSC_EN is for R2R */
> +			csc_en = OVL_CLRFMT_EXT1_CSC_EN(idx);
> +
> +			/*
> +			 * 1. YUV input data - 1 and shift right for 2
> bits to remove it
> +			 * [R']   [0.25    0    0]   [Y in - 1]
> +			 * [G'] = [   0 0.25    0] * [U in - 1]
> +			 * [B']   [   0    0 0.25]   [V in - 1]
> +			 *
> +			 * 2. shift left for 2 bit letting the last 2
> bits become 0
> +			 * [R out]   [ 4  0  0]   [R']
> +			 * [G out] = [ 0  4  0] * [G']
> +			 * [B out]   [ 0  0  4]   [B']
> +			 */
> +		}
> +
> +		mtk_ddp_write_mask(cmdq_pkt, y2r_coef,
> +				   &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_Y2R_PARA_R0(idx),
> +				   OVL_Y2R_PARA_C_CF_RMY);
> +		mtk_ddp_write_mask(cmdq_pkt, (y2r_coef << 16),
> +				   &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_Y2R_PARA_G0(idx),
> +				   OVL_Y2R_PARA_C_CF_GMU);
> +		mtk_ddp_write_mask(cmdq_pkt, y2r_coef,
> +				   &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_Y2R_PARA_B1(idx),
> +				   OVL_Y2R_PARA_C_CF_BMV);
> +
> +		mtk_ddp_write_mask(cmdq_pkt, y2r_offset,
> +				   &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_Y2R_PARA_YUV_A_0(idx),
> +				   OVL_Y2R_PARA_C_CF_YA);
> +		mtk_ddp_write_mask(cmdq_pkt, (y2r_offset << 16),
> +				   &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_Y2R_PARA_YUV_A_0(idx),
> +				   OVL_Y2R_PARA_C_CF_UA);
> +		mtk_ddp_write_mask(cmdq_pkt, y2r_offset,
> +				   &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_Y2R_PARA_YUV_A_1(idx),
> +				   OVL_Y2R_PARA_C_CF_VA);
> +
> +		mtk_ddp_write_relaxed(cmdq_pkt, r2r_coef,
> +				      &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_R2R_R0(idx));
> +		mtk_ddp_write_relaxed(cmdq_pkt, r2r_coef,
> +				      &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_R2R_G1(idx));
> +		mtk_ddp_write_relaxed(cmdq_pkt, r2r_coef,
> +				      &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_R2R_B2(idx));
> +
> +		mtk_ddp_write_mask(cmdq_pkt, csc_en,
> +				   &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_CLRFMT_EXT1,
> +				   OVL_CLRFMT_EXT1_CSC_EN(idx));
> +	}
> +
>  	if (pending->rotation & DRM_MODE_REFLECT_Y) {
>  		con |= OVL_CON_VIRT_FLIP;
>  		addr += (pending->height - 1) * pending->pitch;
CK Hu (胡俊光) Jan. 2, 2024, 7:25 a.m. UTC | #10
Hi, Hsiao-chien:

On Tue, 2023-12-12 at 20:19 +0800, Hsiao Chien Sung wrote:
> Support more color formats in Overlay.
> 
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 7e217142d0c4..a3f1630af5df 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -130,12 +130,20 @@ static const u32 mt8173_formats[] = {
>  static const u32 mt8195_formats[] = {
>  	DRM_FORMAT_XRGB8888,
>  	DRM_FORMAT_ARGB8888,
> +	DRM_FORMAT_XRGB2101010,
>  	DRM_FORMAT_ARGB2101010,
>  	DRM_FORMAT_BGRX8888,
>  	DRM_FORMAT_BGRA8888,
> +	DRM_FORMAT_BGRX1010102,
>  	DRM_FORMAT_BGRA1010102,
>  	DRM_FORMAT_ABGR8888,
>  	DRM_FORMAT_XBGR8888,
> +	DRM_FORMAT_XBGR2101010,
> +	DRM_FORMAT_ABGR2101010,
> +	DRM_FORMAT_RGBX8888,
> +	DRM_FORMAT_RGBA8888,

You does not convert DRM_FORMAT_RGBX8888 and DRM_FORMAT_RGBA8888 to ovl
format. So drop these two format.

Regards,
CK

> +	DRM_FORMAT_RGBX1010102,
> +	DRM_FORMAT_RGBA1010102,
>  	DRM_FORMAT_RGB888,
>  	DRM_FORMAT_BGR888,
>  	DRM_FORMAT_RGB565,
> @@ -431,12 +439,16 @@ static unsigned int ovl_fmt_convert(struct
> mtk_disp_ovl *ovl, unsigned int fmt,
>  		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
>  		       OVL_CON_CLRFMT_ARGB8888 :
>  		       OVL_CON_CLRFMT_PARGB8888;
> +	case DRM_FORMAT_RGBX1010102:
> +	case DRM_FORMAT_RGBA1010102:
> +		return OVL_CON_CLRFMT_ARGB8888;
>  	case DRM_FORMAT_BGRX8888:
>  	case DRM_FORMAT_BGRA8888:
>  		return OVL_CON_BYTE_SWAP |
>  		       (blend_mode == DRM_MODE_BLEND_COVERAGE ?
>  		       OVL_CON_CLRFMT_ARGB8888 :
>  		       OVL_CON_CLRFMT_PARGB8888);
> +	case DRM_FORMAT_BGRX1010102:
>  	case DRM_FORMAT_BGRA1010102:
>  		return OVL_CON_CLRFMT_ARGB8888 | OVL_CON_BYTE_SWAP;
>  	case DRM_FORMAT_XRGB8888:
> @@ -444,6 +456,7 @@ static unsigned int ovl_fmt_convert(struct
> mtk_disp_ovl *ovl, unsigned int fmt,
>  		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
>  		       OVL_CON_CLRFMT_RGBA8888 :
>  		       OVL_CON_CLRFMT_PARGB8888;
> +	case DRM_FORMAT_XRGB2101010:
>  	case DRM_FORMAT_ARGB2101010:
>  		return OVL_CON_CLRFMT_RGBA8888;
>  	case DRM_FORMAT_XBGR8888:
> @@ -452,6 +465,9 @@ static unsigned int ovl_fmt_convert(struct
> mtk_disp_ovl *ovl, unsigned int fmt,
>  		       (blend_mode == DRM_MODE_BLEND_COVERAGE ?
>  		       OVL_CON_CLRFMT_RGBA8888 :
>  		       OVL_CON_CLRFMT_PARGB8888);
> +	case DRM_FORMAT_XBGR2101010:
> +	case DRM_FORMAT_ABGR2101010:
> +		return OVL_CON_CLRFMT_RGBA8888 | OVL_CON_BYTE_SWAP;
>  	case DRM_FORMAT_UYVY:
>  		return OVL_CON_CLRFMT_UYVY | OVL_CON_MTX_YUV_TO_RGB;
>  	case DRM_FORMAT_YUYV:
CK Hu (胡俊光) Jan. 2, 2024, 8:04 a.m. UTC | #11
Hi, Hsiao:

On Tue, 2023-12-12 at 20:19 +0800, Hsiao Chien Sung wrote:
> Register CRC related function pointers to support
> CRC retrieval.
> 
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     | 239
> ++++++++++++++++++++
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |  39 ++++
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   3 +
>  3 files changed, 281 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index bc4cc75cca18..fad728690db7 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -71,6 +71,9 @@ struct mtk_drm_crtc {
>  	/* lock for display hardware access */
>  	struct mutex			hw_lock;
>  	bool				config_updating;
> +
> +	struct mtk_ddp_comp		*crc_provider;
> +	unsigned int			frames;
>  };
>  
>  struct mtk_crtc_state {
> @@ -625,6 +628,14 @@ static void mtk_crtc_ddp_irq(void *data)
>  	struct drm_crtc *crtc = data;
>  	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
>  	struct mtk_drm_private *priv = crtc->dev->dev_private;
> +	struct mtk_ddp_comp *comp = mtk_crtc->crc_provider;
> +
> +	/*
> +	 * crc providers should make sure the crc is always correct
> +	 * by resetting it in .crc_read()
> +	 */
> +	if (crtc->crc.opened)
> +		comp->funcs->crc_read(comp->dev);
>  
>  #if IS_REACHABLE(CONFIG_MTK_CMDQ)
>  	if (!priv->data->shadow_register && !mtk_crtc-
> >cmdq_client.chan)
> @@ -636,6 +647,24 @@ static void mtk_crtc_ddp_irq(void *data)
>  	if (!priv->data->shadow_register)
>  		mtk_crtc_ddp_config(crtc, NULL);
>  #endif
> +
> +	/*
> +	 * drm_crtc_add_crc_entry() could take more than 50ms to finish
> +	 * put it at the end of the isr
> +	 */
> +	if (crtc->crc.opened) {
> +		/*
> +		 * skip the first crc because the first frame is
> configured by
> +		 * mtk_crtc_ddp_hw_init() when atomic enable
> +		 */
> +		if (++mtk_crtc->frames > 1) {
> +			drm_crtc_add_crc_entry(crtc, true,
> +					       drm_crtc_vblank_count(cr
> tc),
> +					       comp->funcs-
> >crc_entry(comp->dev));
> +		}
> +	} else {
> +		mtk_crtc->frames = 0;
> +	}
>  	mtk_drm_finish_page_flip(mtk_crtc);
>  }
>  
> @@ -736,6 +765,40 @@ static int mtk_drm_crtc_update_output(struct
> drm_crtc *crtc,
>  	return 0;
>  }
>  
> +static int mtk_drm_crtc_set_crc_source(struct drm_crtc *crtc, const
> char *src)
> +{
> +	if (src && strcmp(src, "auto") != 0) {
> +		DRM_ERROR("%s(crtc-%d): unknown source '%s'\n",
> +			  __func__, drm_crtc_index(crtc), src);
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +static int mtk_drm_crtc_verify_crc_source(struct drm_crtc *crtc,
> +					  const char *src,
> +					  size_t *cnt)
> +{
> +	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> +	struct mtk_ddp_comp *comp = mtk_crtc->crc_provider;
> +
> +	if (!comp) {
> +		DRM_ERROR("%s(crtc-%d): no crc provider\n",
> +			  __func__, drm_crtc_index(crtc));
> +		return -ENOENT;
> +	}
> +
> +	if (src && strcmp(src, "auto") != 0) {
> +		DRM_ERROR("%s(crtc-%d): unknown source '%s'\n",
> +			  __func__, drm_crtc_index(crtc), src);
> +		return -EINVAL;
> +	}
> +
> +	*cnt = comp->funcs->crc_cnt(comp->dev);
> +
> +	return 0;
> +}
> +
>  int mtk_drm_crtc_plane_check(struct drm_crtc *crtc, struct drm_plane
> *plane,
>  			     struct mtk_plane_state *state)
>  {
> @@ -872,6 +935,8 @@ static const struct drm_crtc_funcs mtk_crtc_funcs
> = {
>  	.atomic_destroy_state	= mtk_drm_crtc_destroy_state,
>  	.enable_vblank		= mtk_drm_crtc_enable_vblank,
>  	.disable_vblank		= mtk_drm_crtc_disable_vblank,
> +	.set_crc_source		= mtk_drm_crtc_set_crc_source,
> +	.verify_crc_source	= mtk_drm_crtc_verify_crc_source,
>  };
>  
>  static const struct drm_crtc_helper_funcs mtk_crtc_helper_funcs = {
> @@ -1073,6 +1138,11 @@ int mtk_drm_crtc_create(struct drm_device
> *drm_dev,
>  
>  			if (comp->funcs->ctm_set)
>  				has_ctm = true;
> +
> +			if (comp->funcs->crc_cnt &&
> +			    comp->funcs->crc_entry &&
> +			    comp->funcs->crc_read)
> +				mtk_crtc->crc_provider = comp;
>  		}
>  
>  		mtk_ddp_comp_register_vblank_cb(comp, mtk_crtc_ddp_irq,
> @@ -1152,3 +1222,172 @@ int mtk_drm_crtc_create(struct drm_device
> *drm_dev,
>  #endif
>  	return 0;
>  }
> +
> +void mtk_drm_crc_init(struct mtk_drm_crc *crc,
> +		      const u32 *crc_offset_table, size_t crc_count,
> +		      u32 reset_offset, u32 reset_mask)
> +{
> +	crc->ofs = crc_offset_table;
> +	crc->cnt = crc_count;
> +	crc->rst_ofs = reset_offset;
> +	crc->rst_msk = reset_mask;
> +	crc->va = kcalloc(crc->cnt, sizeof(*crc->va), GFP_KERNEL);
> +	if (!crc->va) {
> +		DRM_ERROR("failed to allocate memory for crc\n");
> +		crc->cnt = 0;
> +	}
> +}
> +
> +void mtk_drm_crc_read(struct mtk_drm_crc *crc, void __iomem *reg)
> +{
> +	if (!crc->cnt || !crc->ofs || !crc->va)
> +		return;
> +
> +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> +	/* sync to see the most up-to-date copy of the DMA buffer */
> +	dma_sync_single_for_cpu(crc->cmdq_client.chan->mbox->dev,
> +				crc->pa, crc->cnt * sizeof(*crc->va),
> +				DMA_FROM_DEVICE);
> +#else
> +	/* read crc with cpu for the platforms without cmdq */
> +	{
> +		u32 n;
> +
> +		for (n = 0; n < crc->cnt; n++)
> +			crc->va[n] = readl(reg + crc->ofs[n]);
> +
> +		n = readl(reg + crc->rst_ofs);
> +
> +		/* pull reset bit */
> +		n |= crc->rst_msk;
> +		writel(n, reg + crc->rst_ofs);
> +
> +		/* release reset bit */
> +		n &= ~crc->rst_msk;
> +		writel(n, reg + crc->rst_ofs);
> +	}

If CPU has no problem, just use CPU and ignore CMDQ. If CPU has
problem, you should not use CPU.

Regards,
CK

> +#endif
> +}
> +
> +void mtk_drm_crc_destroy(struct mtk_drm_crc *crc)
> +{
> +	if (!crc->cnt)
> +		return;
> +
> +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> +	if (crc->pa) {
> +		dma_unmap_single(crc->cmdq_client.chan->mbox->dev,
> +				 crc->pa, crc->cnt * sizeof(*crc->va),
> +				 DMA_TO_DEVICE);
> +		crc->pa = 0;
> +	}
> +	if (crc->cmdq_client.chan) {
> +		mtk_drm_cmdq_pkt_destroy(&crc->cmdq_handle);
> +		mbox_free_channel(crc->cmdq_client.chan);
> +		crc->cmdq_client.chan = NULL;
> +	}
> +#endif
> +	kfree(crc->va);
> +	crc->va = NULL;
> +	crc->cnt = 0;
> +}
> +
> +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> +void mtk_drm_crc_cmdq_create(struct device *dev, struct mtk_drm_crc
> *crc)
> +{
> +	int i;
> +
> +	if (!crc->cnt) {
> +		dev_warn(dev, "%s: not support\n", __func__);
> +		goto cleanup;
> +	}
> +
> +	if (!crc->ofs) {
> +		dev_warn(dev, "%s: not defined\n", __func__);
> +		goto cleanup;
> +	}
> +
> +	crc->cmdq_client.client.dev = dev;
> +	crc->cmdq_client.client.tx_block = false;
> +	crc->cmdq_client.client.knows_txdone = true;
> +	crc->cmdq_client.client.rx_callback = NULL;
> +	crc->cmdq_client.chan = mbox_request_channel(&crc-
> >cmdq_client.client, 0);
> +	if (IS_ERR(crc->cmdq_client.chan)) {
> +		dev_warn(dev, "%s: failed to create mailbox client\n",
> __func__);
> +		crc->cmdq_client.chan = NULL;
> +		goto cleanup;
> +	}
> +
> +	if (mtk_drm_cmdq_pkt_create(&crc->cmdq_client, &crc-
> >cmdq_handle, PAGE_SIZE)) {
> +		dev_warn(dev, "%s: failed to create cmdq packet\n",
> __func__);
> +		goto cleanup;
> +	}
> +
> +	if (!crc->va) {
> +		dev_warn(dev, "%s: no memory\n", __func__);
> +		goto cleanup;
> +	}
> +
> +	/* map the entry to get a dma address for cmdq to store the crc
> */
> +	crc->pa = dma_map_single(crc->cmdq_client.chan->mbox->dev,
> +				 crc->va, crc->cnt * sizeof(*crc->va),
> +				 DMA_FROM_DEVICE);
> +
> +	if (dma_mapping_error(crc->cmdq_client.chan->mbox->dev, crc-
> >pa)) {
> +		dev_err(dev, "%s: failed to map dma\n", __func__);
> +		goto cleanup;
> +	}
> +
> +	if (crc->cmdq_event)
> +		cmdq_pkt_wfe(&crc->cmdq_handle, crc->cmdq_event, true);
> +
> +	for (i = 0; i < crc->cnt; i++) {
> +		/* put crc to spr1 register */
> +		cmdq_pkt_read_s(&crc->cmdq_handle, crc->cmdq_reg-
> >subsys,
> +				crc->cmdq_reg->offset + crc->ofs[i],
> +				CMDQ_THR_SPR_IDX1);
> +
> +		/* copy spr1 register to physical address of the crc */
> +		cmdq_pkt_assign(&crc->cmdq_handle, CMDQ_THR_SPR_IDX0,
> +				CMDQ_ADDR_HIGH(crc->pa + i *
> sizeof(*crc->va)));
> +		cmdq_pkt_write_s(&crc->cmdq_handle, CMDQ_THR_SPR_IDX0,
> +				 CMDQ_ADDR_LOW(crc->pa + i *
> sizeof(*crc->va)),
> +				 CMDQ_THR_SPR_IDX1);
> +	}
> +	/* reset crc */
> +	mtk_ddp_write_mask(&crc->cmdq_handle, ~0, crc->cmdq_reg, 0,
> +			   crc->rst_ofs, crc->rst_msk);
> +
> +	/* clear reset bit */
> +	mtk_ddp_write_mask(&crc->cmdq_handle, 0, crc->cmdq_reg, 0,
> +			   crc->rst_ofs, crc->rst_msk);
> +
> +	/* jump to head of the cmdq packet */
> +	cmdq_pkt_jump(&crc->cmdq_handle, crc->cmdq_handle.pa_base);
> +
> +	return;
> +cleanup:
> +	mtk_drm_crc_destroy(crc);
> +}
> +
> +void mtk_drm_crc_cmdq_start(struct mtk_drm_crc *crc)
> +{
> +	if (!crc->cmdq_client.chan)
> +		return;
> +
> +	dma_sync_single_for_device(crc->cmdq_client.chan->mbox->dev,
> +				   crc->cmdq_handle.pa_base,
> +				   crc->cmdq_handle.cmd_buf_size,
> +				   DMA_TO_DEVICE);
> +	mbox_send_message(crc->cmdq_client.chan, &crc->cmdq_handle);
> +	mbox_client_txdone(crc->cmdq_client.chan, 0);
> +}
> +
> +void mtk_drm_crc_cmdq_stop(struct mtk_drm_crc *crc)
> +{
> +	if (!crc->cmdq_client.chan)
> +		return;
> +
> +	mbox_flush(crc->cmdq_client.chan, 2000);
> +}
> +#endif
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> index 96790f8f7a94..3440c154ad1e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> @@ -15,6 +15,45 @@
>  #define MTK_MAX_BPC	10
>  #define MTK_MIN_BPC	3
>  
> +/**
> + * struct mtk_drm_crc - crc related information
> + * @ofs: register offset of crc
> + * @rst_ofs: register offset of crc reset
> + * @rst_msk: register mask of crc reset
> + * @cnt: count of crc
> + * @va: pointer to the start of crc array
> + * @pa: physical address of the crc for gce to access
> + * @cmdq_event: the event to trigger the cmdq
> + * @cmdq_reg: address of the register that cmdq is going to access
> + * @cmdq_client: handler to control cmdq (mbox channel, thread
> ...etc.)
> + * @cmdq_handle: cmdq packet to store the commands
> + */
> +struct mtk_drm_crc {
> +	const u32 *ofs;
> +	u32 rst_ofs;
> +	u32 rst_msk;
> +	size_t cnt;
> +	u32 *va;
> +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> +	dma_addr_t pa;
> +	u32 cmdq_event;
> +	struct cmdq_client_reg *cmdq_reg;
> +	struct cmdq_client cmdq_client;
> +	struct cmdq_pkt cmdq_handle;
> +#endif
> +};
> +
> +void mtk_drm_crc_init(struct mtk_drm_crc *crc,
> +		      const u32 *crc_offset_table, size_t crc_count,
> +		      u32 reset_offset, u32 reset_mask);
> +void mtk_drm_crc_read(struct mtk_drm_crc *crc, void __iomem *reg);
> +void mtk_drm_crc_destroy(struct mtk_drm_crc *crc);
> +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> +void mtk_drm_crc_cmdq_create(struct device *dev, struct mtk_drm_crc
> *crc);
> +void mtk_drm_crc_cmdq_start(struct mtk_drm_crc *crc);
> +void mtk_drm_crc_cmdq_stop(struct mtk_drm_crc *crc);
> +#endif
> +
>  void mtk_drm_crtc_commit(struct drm_crtc *crtc);
>  int mtk_drm_crtc_create(struct drm_device *drm_dev,
>  			const unsigned int *path,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index 2597dd7ac0d2..38d08796fae4 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -84,6 +84,9 @@ struct mtk_ddp_comp_funcs {
>  	void (*add)(struct device *dev, struct mtk_mutex *mutex);
>  	void (*remove)(struct device *dev, struct mtk_mutex *mutex);
>  	int (*encoder_index)(struct device *dev);
> +	size_t (*crc_cnt)(struct device *dev);
> +	u32 *(*crc_entry)(struct device *dev);
> +	void (*crc_read)(struct device *dev);
>  };
>  
>  struct mtk_ddp_comp {
CK Hu (胡俊光) Jan. 2, 2024, 8:26 a.m. UTC | #12
Hi, Hsiao-chien:

On Tue, 2023-12-12 at 20:19 +0800, Hsiao Chien Sung wrote:
> Create rotation property according to the hardware capability.
> Since currently OVL of all chips support same rotation,
> no need to define it in the driver data.
> 
> Fixes: 84d805753983 ("drm/mediatek: Support reflect-y plane
> rotation")
> 
> Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_drv.h        |  1 +
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c        | 18 ++++++--------
> ----
>  .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c    |  9 +++++++++
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c    |  1 +
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c       |  2 +-
>  5 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> index 4d6e8b667bc3..c5afeb7c5527 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -127,6 +127,7 @@ void mtk_ovl_adaptor_register_vblank_cb(struct
> device *dev, void (*vblank_cb)(vo
>  void mtk_ovl_adaptor_unregister_vblank_cb(struct device *dev);
>  void mtk_ovl_adaptor_enable_vblank(struct device *dev);
>  void mtk_ovl_adaptor_disable_vblank(struct device *dev);
> +unsigned int mtk_ovl_adaptor_supported_rotations(struct device
> *dev);
>  void mtk_ovl_adaptor_start(struct device *dev);
>  void mtk_ovl_adaptor_stop(struct device *dev);
>  unsigned int mtk_ovl_adaptor_layer_nr(struct device *dev);
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 78749dabeb43..f019737078f6 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -399,6 +399,10 @@ unsigned int mtk_ovl_layer_nr(struct device
> *dev)
>  
>  unsigned int mtk_ovl_supported_rotations(struct device *dev)
>  {
> +	/*
> +	 * although currently OVL can only do reflection,
> +	 * reflect x + reflect y = rotate 180
> +	 */
>  	return DRM_MODE_ROTATE_0 | DRM_MODE_ROTATE_180 |
>  	       DRM_MODE_REFLECT_X | DRM_MODE_REFLECT_Y;
>  }
> @@ -407,27 +411,17 @@ int mtk_ovl_layer_check(struct device *dev,
> unsigned int idx,
>  			struct mtk_plane_state *mtk_state)
>  {
>  	struct drm_plane_state *state = &mtk_state->base;
> -	unsigned int rotation = 0;
>  
> -	rotation = drm_rotation_simplify(state->rotation,
> -					 DRM_MODE_ROTATE_0 |
> -					 DRM_MODE_REFLECT_X |
> -					 DRM_MODE_REFLECT_Y);
> -	rotation &= ~DRM_MODE_ROTATE_0;
> -
> -	/* We can only do reflection, not rotation */
> -	if ((rotation & DRM_MODE_ROTATE_MASK) != 0)
> +	if (state->rotation & ~mtk_ovl_supported_rotations(dev))
>  		return -EINVAL;
>  
>  	/*
>  	 * TODO: Rotating/reflecting YUV buffers is not supported at
> this time.
>  	 *	 Only RGB[AX] variants are supported.
>  	 */
> -	if (state->fb->format->is_yuv && rotation != 0)
> +	if (state->fb->format->is_yuv && (state->rotation &
> ~DRM_MODE_ROTATE_0))

You still no explain what you do here.

>  		return -EINVAL;
>  
> -	state->rotation = rotation;
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> index 4398db9a6276..273c79d37bef 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> @@ -383,6 +383,15 @@ void mtk_ovl_adaptor_register_vblank_cb(struct
> device *dev, void (*vblank_cb)(vo
>  				     vblank_cb, vblank_cb_data);
>  }
>  
> +unsigned int mtk_ovl_adaptor_supported_rotations(struct device *dev)
> +{
> +	/*
> +	 * should still return DRM_MODE_ROTATE_0 if rotation is not
> supported,
> +	 * or IGT will fail.
> +	 */
> +	return DRM_MODE_ROTATE_0;
> +}
> +
>  void mtk_ovl_adaptor_unregister_vblank_cb(struct device *dev)
>  {
>  	struct mtk_disp_ovl_adaptor *ovl_adaptor =
> dev_get_drvdata(dev);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index ffa4868b1222..206dd6f6f99e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -422,6 +422,7 @@ static const struct mtk_ddp_comp_funcs
> ddp_ovl_adaptor = {
>  	.remove = mtk_ovl_adaptor_remove_comp,
>  	.get_formats = mtk_ovl_adaptor_get_formats,
>  	.get_num_formats = mtk_ovl_adaptor_get_num_formats,
> +	.supported_rotations = mtk_ovl_adaptor_supported_rotations,
>  };
>  
>  static const char * const mtk_ddp_comp_stem[MTK_DDP_COMP_TYPE_MAX] =
> {
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> index ff300426b590..e73b9793dee2 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -343,7 +343,7 @@ int mtk_plane_init(struct drm_device *dev, struct
> drm_plane *plane,
>  		return err;
>  	}
>  
> -	if (supported_rotations & ~DRM_MODE_ROTATE_0) {
> +	if (supported_rotations) {

Try report issue to IGT team.

Regards,
CK

>  		err = drm_plane_create_rotation_property(plane,
>  							 DRM_MODE_ROTAT
> E_0,
>  							 supported_rota
> tions);
AngeloGioacchino Del Regno Feb. 1, 2024, 10:16 a.m. UTC | #13
Il 12/12/23 13:19, Hsiao Chien Sung ha scritto:
> This series is based on mediatek-drm-next.
> 
> This series adds support for running IGT (Intel GPU Tool) tests
> with MediaTek display driver. The following changes will be
> applied:
> 
> 1. Add a new API for creating GCE thread loop to retrieve CRCs
>     from the hardware component
> 2. Support hardware CRC calculation in both VDOSYS0 and VDOSYS1
> 3. Support alpha blending in both VDOSYS0 and VDOSYS1
> 

Hello,
is there still interest in upstreaming this?

We're interested in enabling more comprehensive IGT tests on MediaTek SoCs
and this series is definitely useful.

Regards,
Angelo

> Changes in v4:
> - Seperate the patch into smaller ones
> - Change the title of some patches
> - Revert the changes that are not related to the series
> 
> Changes in v3:
> - Modify the dt-binding document of Mediatek OVL
> - Set DRM mode configs accroding to the hardware capabilities
> - Replace cmdq_pkt_jump_absolute() with cmdq_pkt_jump()
> 
> Changes in v2:
> - Simplify CMDQ by adding commands that are currently used only
> - Integrate CRC related codes into new APIs for Mixer and OVL to reuse
> - Add CPU version CRC retrieval when CMDQ is disabled
> 
> Hsiao Chien Sung (17):
>    soc: mediatek: Add register definitions for GCE
>    soc: mediatek: Disable 9-bit alpha in ETHDR
>    dt-bindings: display: mediatek: ovl: Modify rules for MT8195/MT8188
>    drm/mediatek: Add OVL compatible name for MT8195
>    drm/mediatek: Set DRM mode configs accordingly
>    drm/mediatek: Support alpha blending in OVL
>    drm/mediatek: Support alpha blending in Mixer
>    drm/mediatek: Support alpha blending in display driver
>    drm/mediatek: Support CSC in OVL
>    drm/mediatek: Support more color formats in OVL
>    drm/mediatek: Turn off the layers with zero width or height
>    drm/mediatek: Support CRC in display driver
>    drm/mediatek: Support CRC in OVL
>    drm/mediatek: Support CRC in OVL adaptor
>    drm/mediatek: Add missing plane settings when async update
>    drm/mediatek: Fix errors when reporting rotation capability
>    drm/mediatek: Add comments for the structures
> 
>   .../display/mediatek/mediatek,ovl.yaml        |  12 +-
>   drivers/gpu/drm/mediatek/mtk_disp_drv.h       |   7 +
>   drivers/gpu/drm/mediatek/mtk_disp_ovl.c       | 326 +++++++++++++++---
>   .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c   |  32 +-
>   drivers/gpu/drm/mediatek/mtk_drm_crtc.c       | 261 +++++++++++++-
>   drivers/gpu/drm/mediatek/mtk_drm_crtc.h       |  39 +++
>   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c   |   7 +
>   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  35 ++
>   drivers/gpu/drm/mediatek/mtk_drm_drv.c        |  30 +-
>   drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  16 +
>   drivers/gpu/drm/mediatek/mtk_drm_plane.c      |  15 +-
>   drivers/gpu/drm/mediatek/mtk_ethdr.c          | 106 +++++-
>   drivers/gpu/drm/mediatek/mtk_ethdr.h          |   5 +
>   drivers/soc/mediatek/mtk-mmsys.c              |   1 +
>   include/linux/soc/mediatek/mtk-cmdq.h         |  10 +
>   15 files changed, 834 insertions(+), 68 deletions(-)
> 
> --
> 2.18.0
>
Shawn Sung Feb. 2, 2024, 9:03 a.m. UTC | #14
Hi Angelo,

Thanks for the reminder.
The next version is expected to be released next week.

Regards,
Shawn

The next version will 
On Thu, 2024-02-01 at 11:16 +0100, AngeloGioacchino Del Regno wrote:
> Il 12/12/23 13:19, Hsiao Chien Sung ha scritto:
> > This series is based on mediatek-drm-next.
> > 
> > This series adds support for running IGT (Intel GPU Tool) tests
> > with MediaTek display driver. The following changes will be
> > applied:
> > 
> > 1. Add a new API for creating GCE thread loop to retrieve CRCs
> >     from the hardware component
> > 2. Support hardware CRC calculation in both VDOSYS0 and VDOSYS1
> > 3. Support alpha blending in both VDOSYS0 and VDOSYS1
> > 
> 
> Hello,
> is there still interest in upstreaming this?
> 
> We're interested in enabling more comprehensive IGT tests on MediaTek
> SoCs
> and this series is definitely useful.
> 
> Regards,
> Angelo
> 
> > Changes in v4:
> > - Seperate the patch into smaller ones
> > - Change the title of some patches
> > - Revert the changes that are not related to the series
> > 
> > Changes in v3:
> > - Modify the dt-binding document of Mediatek OVL
> > - Set DRM mode configs accroding to the hardware capabilities
> > - Replace cmdq_pkt_jump_absolute() with cmdq_pkt_jump()
> > 
> > Changes in v2:
> > - Simplify CMDQ by adding commands that are currently used only
> > - Integrate CRC related codes into new APIs for Mixer and OVL to
> > reuse
> > - Add CPU version CRC retrieval when CMDQ is disabled
> > 
> > Hsiao Chien Sung (17):
> >    soc: mediatek: Add register definitions for GCE
> >    soc: mediatek: Disable 9-bit alpha in ETHDR
> >    dt-bindings: display: mediatek: ovl: Modify rules for
> > MT8195/MT8188
> >    drm/mediatek: Add OVL compatible name for MT8195
> >    drm/mediatek: Set DRM mode configs accordingly
> >    drm/mediatek: Support alpha blending in OVL
> >    drm/mediatek: Support alpha blending in Mixer
> >    drm/mediatek: Support alpha blending in display driver
> >    drm/mediatek: Support CSC in OVL
> >    drm/mediatek: Support more color formats in OVL
> >    drm/mediatek: Turn off the layers with zero width or height
> >    drm/mediatek: Support CRC in display driver
> >    drm/mediatek: Support CRC in OVL
> >    drm/mediatek: Support CRC in OVL adaptor
> >    drm/mediatek: Add missing plane settings when async update
> >    drm/mediatek: Fix errors when reporting rotation capability
> >    drm/mediatek: Add comments for the structures
> > 
> >   .../display/mediatek/mediatek,ovl.yaml        |  12 +-
> >   drivers/gpu/drm/mediatek/mtk_disp_drv.h       |   7 +
> >   drivers/gpu/drm/mediatek/mtk_disp_ovl.c       | 326
> > +++++++++++++++---
> >   .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c   |  32 +-
> >   drivers/gpu/drm/mediatek/mtk_drm_crtc.c       | 261
> > +++++++++++++-
> >   drivers/gpu/drm/mediatek/mtk_drm_crtc.h       |  39 +++
> >   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c   |   7 +
> >   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  35 ++
> >   drivers/gpu/drm/mediatek/mtk_drm_drv.c        |  30 +-
> >   drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  16 +
> >   drivers/gpu/drm/mediatek/mtk_drm_plane.c      |  15 +-
> >   drivers/gpu/drm/mediatek/mtk_ethdr.c          | 106 +++++-
> >   drivers/gpu/drm/mediatek/mtk_ethdr.h          |   5 +
> >   drivers/soc/mediatek/mtk-mmsys.c              |   1 +
> >   include/linux/soc/mediatek/mtk-cmdq.h         |  10 +
> >   15 files changed, 834 insertions(+), 68 deletions(-)
> > 
> > --
> > 2.18.0
> > 
> 
>