linux-kernel - Re: [PATCH v3,2/2] drm: mediatek: Adjust the dpi output format to MT8186

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 23 Aug 2022 16:16:22 -0400
From:   Nícolas F. R. A. Prado 
        <nfraprado@...labora.com>
To:     xinlei.lee@...iatek.com
Cc:     chunkuang.hu@...nel.org, p.zabel@...gutronix.de, airlied@...ux.ie,
        daniel@...ll.ch, matthias.bgg@...il.com, rex-bc.chen@...iatek.com,
        angelogioacchino.delregno@...labora.com, jason-jh.lin@...iatek.com,
        yongqiang.niu@...iatek.com, dri-devel@...ts.freedesktop.org,
        linux-mediatek@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Project_Global_Chrome_Upstream_Group@...iatek.com,
        Jitao Shi <jitao.shi@...iatek.com>
Subject: Re: [PATCH v3,2/2] drm: mediatek: Adjust the dpi output format to
 MT8186

On Tue, Aug 23, 2022 at 02:18:37PM +0800, xinlei.lee@...iatek.com wrote:
> From: Xinlei Lee <xinlei.lee@...iatek.com>
> 
> Dpi output needs to adjust the output format to dual edge for MT8186.
> Because MT8186 HW has been modified at that time, SW needs to cooperate.
> And the register (MMSYS) reserved for dpi will be used for output
> format control (dual_edge/single_edge).
> 
> Co-developed-by: Jitao Shi <jitao.shi@...iatek.com>
> Signed-off-by: Jitao Shi <jitao.shi@...iatek.com>
> Signed-off-by: Xinlei Lee <xinlei.lee@...iatek.com>
> 
> ---
[..]
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
[..]
>   * @yuv422_en_bit: Enable bit of yuv422.
>   * @csc_enable_bit: Enable bit of CSC.
>   * @pixels_per_iter: Quantity of transferred pixels per iteration.
> + * @rgb888_dual_enable: Control output format for mt8186.

Let's not mention mt8186 in the description to keep the property generic. Also,
this description should say what having 'rgb888_dual_enable = true' indicates
about the hardware (in this case mt8186) and it currently doesn't.

Let's take a step back. What does 'dual enable' mean in this context and how
does it relate to 'dual edge' and the dpi output format? By answering those
questions we can find a description (and maybe variable name) that makes more
sense.

>   */
[..]
> @@ -449,6 +454,9 @@ static void mtk_dpi_dual_edge(struct mtk_dpi *dpi)
>  		mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING,
>  			     dpi->output_fmt == MEDIA_BUS_FMT_RGB888_2X12_LE ?
>  			     EDGE_SEL : 0, EDGE_SEL);
> +	if (dpi->conf->rgb888_dual_enable)
> +		mtk_mmsys_ddp_dpi_fmt_config(dpi->mmsys_dev, DPI_RGB888_DDR_CON,
> +					     DPI_FORMAT_MASK, NULL);

This if block should be further indented.

>  	} else {
>  		mtk_dpi_mask(dpi, DPI_DDR_SETTING, DDR_EN | DDR_4PHASE, 0);
>  	}
[..]
> --- a/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi_regs.h
> @@ -235,4 +235,8 @@
>  #define MATRIX_SEL_RGB_TO_JPEG		0
>  #define MATRIX_SEL_RGB_TO_BT601		2
>  
> +#define DPI_FORMAT_MASK			0x1
> +#define DPI_RGB888_DDR_CON		BIT(0)
> +#define DPI_RGB565_SDR_CON		BIT(1)

I'm not sure if it would make more sense to have these definitions in the mmsys
header since they're configurations of a register in mmsys' iospace... I think
we can keep them here but at least add a comment above:

/* Values for DPI configuration in MMSYS address space */

Thanks,
Nícolas