On Fri, Jan 29, 2021 at 3:42 PM Yongqiang Niu yongqiang.niu@mediatek.com wrote:
On Fri, 2021-01-29 at 14:46 +0800, Hsin-Yi Wang wrote:
On Fri, Jan 29, 2021 at 2:30 PM Yongqiang Niu yongqiang.niu@mediatek.com wrote:
On Fri, 2021-01-29 at 14:24 +0800, Hsin-Yi Wang wrote:
On Fri, Jan 29, 2021 at 9:33 AM CK Hu ck.hu@mediatek.com wrote:
Hi, Hsin-Yi:
On Thu, 2021-01-28 at 19:23 +0800, Hsin-Yi Wang wrote:
From: Yongqiang Niu yongqiang.niu@mediatek.com
for 5 or 6 bpc panel, we need enable dither function to improve the display quality
Signed-off-by: Yongqiang Niu yongqiang.niu@mediatek.com Signed-off-by: Hsin-Yi Wang hsinyi@chromium.org
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c index ac2cb25620357..6c8f246380a74 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c @@ -53,6 +53,7 @@ #define DITHER_EN BIT(0) #define DISP_DITHER_CFG 0x0020 #define DITHER_RELAY_MODE BIT(0) +#define DITHER_ENGINE_EN BIT(1) #define DISP_DITHER_SIZE 0x0030
#define LUT_10BIT_MASK 0x03ff @@ -314,9 +315,19 @@ static void mtk_dither_config(struct device *dev, unsigned int w, unsigned int bpc, struct cmdq_pkt *cmdq_pkt) { struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
bool enable = (bpc == 5 || bpc == 6);I strongly believe that dither function in dither is identical to the one in gamma and od, and in mtk_dither_set_common(), 'bpc >= MTK_MIN_BPC' is valid, so I believe we need not to limit bpc to 5 or 6. But we should consider the case that bpc is invalid in mtk_dither_set_common(). Invalid case in gamma and od use different way to process. For gamma, dither is default relay mode, so invalid bpc would do nothing in mtk_dither_set_common() and result in relay mode. For od, it set to relay mode first, them invalid bpc would do nothing in mtk_dither_set_common() and result in relay mode. I would like dither, gamma and od to process invalid bpc in the same way. One solution is to set relay mode in mtk_dither_set_common() for invalid bpc.
Regards, CK
I modify the mtk_dither_config() to follow:
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c index ac2cb25620357..5b7fcedb9f9a8 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c @@ -53,6 +53,7 @@ #define DITHER_EN BIT(0) #define DISP_DITHER_CFG 0x0020 #define DITHER_RELAY_MODE BIT(0) +#define DITHER_ENGINE_EN BIT(1) #define DISP_DITHER_SIZE 0x0030
#define LUT_10BIT_MASK 0x03ff @@ -166,6 +167,8 @@ void mtk_dither_set_common(void __iomem *regs, struct cmdq_client_reg *cmdq_reg, DITHER_ADD_LSHIFT_G(MTK_MAX_BPC - bpc), cmdq_reg, regs, DISP_DITHER_16); mtk_ddp_write(cmdq_pkt, dither_en, cmdq_reg, regs, cfg);
} else {mtk_ddp_write(cmdq_pkt, DITHER_RELAY_MODE, cmdq_reg, regs, cfg); }}
@@ -315,8 +318,12 @@ static void mtk_dither_config(struct device *dev, unsigned int w, { struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
mtk_ddp_write(cmdq_pkt, h << 16 | w, &priv->cmdq_reg,priv->regs, DISP_DITHER_SIZE);
mtk_ddp_write(cmdq_pkt, DITHER_RELAY_MODE, &priv->cmdq_reg,priv->regs, DISP_DITHER_CFG);
mtk_ddp_write(cmdq_pkt, h << 16 | w, &priv->cmdq_reg, priv->regs,DISP_DITHER_SIZE);mtk_ddp_write(cmdq_pkt, DITHER_RELAY_MODE, &priv->cmdq_reg, priv->regs,DISP_DITHER_CFG);mtk_dither_set_common(priv->regs, &priv->cmdq_reg, bpc, DISP_DITHER_CFG,DITHER_ENGINE_EN, cmdq_pkt);}
So now, not only bpc==5 or 6, but all valid bpc, dither config will call mtk_dither_set_common() with the flag DITHER_ENGINE_EN(BIT(1)). od config will call mtk_dither_set_common() with the flag DISP_DITHERING(BIT(2)). Additionally for 8173, gamma config will call mtk_dither_set_common() with the flag DISP_DITHERING (BIT(2))
For invalid mode all of them will be DITHER_RELAY_MODE.
Just to make sure that this follows the spec? thanks
for mt8173 gamma, there is no relay mode, only dither enable or not(bit 2). for mt8183 dither, there is dither enable bit 1, and relay mode bit 0
CK suggested to set relay mode for invalid cases. Or should I just set invalid case in mtk_dither_config()? So that invalid case for gamma and od would remain its default mode?
od and gamma has no relay mode set relay mode in mtk_dither_config is better
Besides that, the major difference of this patch and original version is that not only bpc ==5 or 6 will set dither enable bit 1. Does this looks good to you?
dither only support bpc 4 6 8 , there is no bpc 5 use case, please modify this error.
Only bpc 4, 6, 8 case will call mtk_dither_set_common, other bpc will be considered invalid and stays in relay mode.
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c index ac2cb25620357..5761dd15eedf2 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c @@ -53,6 +53,7 @@ #define DITHER_EN BIT(0) #define DISP_DITHER_CFG 0x0020 #define DITHER_RELAY_MODE BIT(0) +#define DITHER_ENGINE_EN BIT(1) #define DISP_DITHER_SIZE 0x0030
#define LUT_10BIT_MASK 0x03ff @@ -314,9 +315,17 @@ static void mtk_dither_config(struct device *dev, unsigned int w, unsigned int bpc, struct cmdq_pkt *cmdq_pkt) { struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev); - - mtk_ddp_write(cmdq_pkt, h << 16 | w, &priv->cmdq_reg, priv->regs, DISP_DITHER_SIZE); - mtk_ddp_write(cmdq_pkt, DITHER_RELAY_MODE, &priv->cmdq_reg, priv->regs, DISP_DITHER_CFG); + bool valid_bpc = (bpc == 4 || bpc == 6 || bpc == 8); + + mtk_ddp_write(cmdq_pkt, h << 16 | w, &priv->cmdq_reg, priv->regs, + DISP_DITHER_SIZE); + if (valid_bpc)
+ mtk_dither_set_common(priv->regs, &priv->cmdq_reg, bpc, + DISP_DITHER_CFG, DITHER_ENGINE_EN, + cmdq_pkt); + else + mtk_ddp_write(cmdq_pkt, DITHER_RELAY_MODE, &priv->cmdq_reg, + priv->regs, DISP_DITHER_CFG); }
mtk_ddp_write(cmdq_pkt, h << 16 | w, &priv->cmdq_reg, priv->regs, DISP_DITHER_SIZE);mtk_ddp_write(cmdq_pkt, DITHER_RELAY_MODE, &priv->cmdq_reg, priv->regs, DISP_DITHER_CFG);
if (enable) {mtk_dither_set_common(priv->regs, &priv->cmdq_reg, bpc,DISP_DITHER_CFG, DITHER_ENGINE_EN,cmdq_pkt);} else {mtk_ddp_write(cmdq_pkt, DITHER_RELAY_MODE, &priv->cmdq_reg,priv->regs, DISP_DITHER_CFG);}mtk_ddp_write(cmdq_pkt, h << 16 | w, &priv->cmdq_reg, priv->regs,DISP_DITHER_SIZE);}
static void mtk_dither_start(struct device *dev)