On Wed, 2025-02-12 at 03:45 +0000, CK Hu (胡俊光) wrote:
Hi, Paul:On Fri, 2025-01-10 at 20:34 +0800, paul-pl.chen wrote:> From: "Nancy.Lin" <nancy.lin@xxxxxxxxxxxx>>> BLENDER executes the alpha blending function for overlapping> layers from different sources, which is the primary function> of the overlapping system.>> Signed-off-by: Nancy.Lin <nancy.lin@xxxxxxxxxxxx>> Signed-off-by: Paul-pl.Chen <paul-pl.chen@xxxxxxxxxxxx>> ---> drivers/gpu/drm/mediatek/Makefile | 1 +> drivers/gpu/drm/mediatek/mtk_ddp_comp.c | 1 +> drivers/gpu/drm/mediatek/mtk_ddp_comp.h | 1 +> drivers/gpu/drm/mediatek/mtk_disp_blender.c | 352 ++++++++++++++++++++> drivers/gpu/drm/mediatek/mtk_disp_blender.h | 17 +> drivers/gpu/drm/mediatek/mtk_disp_drv.h | 12 +> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 +> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 1 +> 8 files changed, 386 insertions(+)> create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_blender.c> create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_blender.h>
//snip
> +static inline bool is_10bit_rgb(u32 fmt)This function is the same as the one in OVL driver. Try to merge them.
OK we will try to merge them.
> +{> +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 unsigned int mtk_disp_blender_fmt_convert(unsigned int fmt, unsigned int blend_mode)This function is similar to mtk_ovl_fmt_convert(), try to merge them.
We will review function of "mtk_disp_blender_fmt_convert" and try to merge it.
> +{> +/*> + * DRM_FORMAT: bit 32->0, BLD_FMT: bit 0->32,> + * so DRM_FORMAT_RGB888 = OVL_BLD_CON_CLRFMT_BGR888> + */
// snip
> +void mtk_disp_blender_layer_config(struct device *dev, struct mtk_plane_state *state,> + struct cmdq_pkt *cmdq_pkt)> +{> +struct mtk_disp_blender *priv = dev_get_drvdata(dev);> +struct mtk_plane_pending_state *pending = &state->pending;> +unsigned int align_width = ALIGN_DOWN(pending->width, 2);Why do this alignment?If width is 101, changing it to 102 would get correct display output?I does not see pitch setting in this driver, so this hardware does not support pitch?
Sure, we will check this part.
> +unsigned int alpha;> +unsigned int clrfmt;> +unsigned int blend_mode = DRM_MODE_BLEND_PIXEL_NONE;> +> +if (!pending->enable) {> +mtk_ddp_write_mask(cmdq_pkt, 0, &priv->cmdq_reg, priv->regs,> + DISP_REG_OVL_BLD_L_EN, OVL_BLD_L_EN);> +return;> +}> +> +mtk_ddp_write(cmdq_pkt, pending->y << 16 | pending->x, &priv->cmdq_reg, priv->regs,> + DISP_REG_BLD_OVL_OFFSET);> +> +mtk_ddp_write(cmdq_pkt, pending->height << 16 | align_width, &priv->cmdq_reg, priv->regs,> + DISP_REG_BLD_OVL_SRC_SIZE);> +> +if (state->base.fb && state->base.fb->format->has_alpha)> +blend_mode = state->base.pixel_blend_mode;> +> +clrfmt = mtk_disp_blender_fmt_convert(pending->format, blend_mode);> +> +mtk_ddp_write_mask(cmdq_pkt, clrfmt, &priv->cmdq_reg, priv->regs,> + DISP_REG_OVL_BLD_L0_CLRFMT, OVL_BLD_CON_CLRFMT_MAN |> + OVL_BLD_CON_RGB_SWAP | OVL_BLD_CON_BYTE_SWAP |> + OVL_BLD_CON_FLD_CLRFMT | OVL_BLD_CON_FLD_CLRFMT_NB);> +> +alpha = (0xFF & (state->base.alpha >> 8)) | OVL_BLD_L_ALPHA_EN;I think state->base.alpha is in the range of 0 ~ 255.Do you test alpha blending with this patch?If no, it's better to remove alpha related setting in this patch.After you have test alpha, send alpha related patch.
Yes, we have done the test alpha blending with this patch,
I will check the driver what you mentioned.
I will check the driver what you mentioned.
I think this hardware has the same behavior with OVL hardware,So I would like alpha related code is similar with the code in ovl driver.> +mtk_ddp_write_mask(cmdq_pkt, alpha, &priv->cmdq_reg, priv->regs,> + DISP_REG_OVL_BLD_L_CON2, OVL_BLD_L_ALPHA_EN | OVL_BLD_L_ALPHA);> +> +mtk_ddp_write_mask(cmdq_pkt, OVL_BLD_L_EN, &priv->cmdq_reg, priv->regs,> + DISP_REG_OVL_BLD_L_EN, OVL_BLD_L_EN);Move enable hardware to the bottom of this function.
OK I will follow to suggestion to set all the configuration before enable.
> +> +if (blend_mode == DRM_MODE_BLEND_PIXEL_NONE)> +mtk_ddp_write_mask(cmdq_pkt, OVL_L0_CONST_BLD, &priv->cmdq_reg, priv->regs,> + DISP_REG_OVL_BLD_L0_PITCH, OVL_L0_CONST_BLD);> +else> +mtk_ddp_write_mask(cmdq_pkt, 0, &priv->cmdq_reg, priv->regs,> + DISP_REG_OVL_BLD_L0_PITCH, OVL_L0_CONST_BLD);> +}> +> +void mtk_disp_blender_config(struct device *dev, unsigned int w,> + unsigned int h, unsigned int vrefresh,> + unsigned int bpc, enum mtk_disp_blender_layer blender,> + struct cmdq_pkt *cmdq_pkt)> +{> +struct mtk_disp_blender *priv = dev_get_drvdata(dev);> +unsigned int tmp;> +> +dev_dbg(dev, "%s-w:%d, h:%d\n", __func__, w, h);> +> +tmp = readl(priv->regs + DISP_REG_OVL_BLD_SHADOW_CTRL);> +tmp = tmp | DISP_OVL_BLD_BYPASS_SHADOW;> +writel(tmp, priv->regs + DISP_REG_OVL_BLD_SHADOW_CTRL);Move bypass shadow to mtk_disp_blender_start().
Sure, I will move bypass shadow setting to tk_disp_blender_start to align the driver.
> +> +mtk_ddp_write(cmdq_pkt, h << 16 | w, &priv->cmdq_reg, priv->regs,> + DISP_REG_OVL_BLD_ROI_SIZE);> +mtk_ddp_write(cmdq_pkt, DISP_OVL_BLD_BGCLR_BALCK, &priv->cmdq_reg, priv->regs,> + DISP_REG_OVL_BLD_BGCLR_CLR);> +> +if (blender == FIRST_BLENDER)> +mtk_ddp_write_mask(cmdq_pkt, OVL_BLD_BGCLR_OUT_TO_NEXT_LAYER,> + &priv->cmdq_reg, priv->regs, DISP_REG_OVL_BLD_DATAPATH_CON,> + OVL_BLD_BGCLR_OUT_TO_PROC | OVL_BLD_BGCLR_OUT_TO_NEXT_LAYER |> + OVL_BLD_BGCLR_IN_SEL);> +else if (blender == LAST_BLENDER)> +mtk_ddp_write_mask(cmdq_pkt, OVL_BLD_BGCLR_OUT_TO_PROC | OVL_BLD_BGCLR_IN_SEL,> + &priv->cmdq_reg, priv->regs, DISP_REG_OVL_BLD_DATAPATH_CON,> + OVL_BLD_BGCLR_OUT_TO_PROC | OVL_BLD_BGCLR_OUT_TO_NEXT_LAYER |> + OVL_BLD_BGCLR_IN_SEL);> +else if (blender == SINGLE_BLENDER)> +mtk_ddp_write_mask(cmdq_pkt, OVL_BLD_BGCLR_OUT_TO_PROC,> + &priv->cmdq_reg, priv->regs, DISP_REG_OVL_BLD_DATAPATH_CON,> + OVL_BLD_BGCLR_OUT_TO_PROC | OVL_BLD_BGCLR_OUT_TO_NEXT_LAYER |> + OVL_BLD_BGCLR_IN_SEL);> +else> +mtk_ddp_write_mask(cmdq_pkt, OVL_BLD_BGCLR_OUT_TO_NEXT_LAYER | OVL_BLD_BGCLR_IN_SEL,> + &priv->cmdq_reg, priv->regs, DISP_REG_OVL_BLD_DATAPATH_CON,> + OVL_BLD_BGCLR_OUT_TO_PROC | OVL_BLD_BGCLR_OUT_TO_NEXT_LAYER |> + OVL_BLD_BGCLR_IN_SEL);I would like to drop blender, andif (!first_blender)bld_bgclr |= OVL_BLD_BGCLR_IN_SEL;if (last_blender)bld_bgclr |= OVL_BLD_BGCLR_OUT_TO_PROC;elsebld_bgclr |= OVL_BLD_BGCLR_OUT_TO_NEXT_LAYER;mtk_ddp_write_mask(cmdq_pkt, bld_bgclr,OVL_BLD_BGCLR_OUT_TO_PROC | OVL_BLD_BGCLR_OUT_TO_NEXT_LAYER |OVL_BLD_BGCLR_IN_SEL);The caller would call like this:mtk_disp_blender_config(..., blender_idx == 1, blender_idx == layer_nr)
Ok, we will follow the suggestion to make blender layer
> +}> +> +void mtk_disp_blender_start(struct device *dev, struct cmdq_pkt *cmdq_pkt)> +{> +struct mtk_disp_blender *priv = dev_get_drvdata(dev);> +> +mtk_ddp_write_mask(cmdq_pkt, OVL_BLD_EN, &priv->cmdq_reg, priv->regs,> + DISP_REG_OVL_BLD_EN, OVL_BLD_EN);> +}> +> +void mtk_disp_blender_stop(struct device *dev, struct cmdq_pkt *cmdq_pkt)> +{> +struct mtk_disp_blender *priv = dev_get_drvdata(dev);> +> +mtk_ddp_write_mask(cmdq_pkt, 0, &priv->cmdq_reg, priv->regs,> + DISP_REG_OVL_BLD_L_EN, OVL_BLD_L_EN);When OVL stop, it does not disable layer.So align the behavior with OVL driver.Regards,CK
Sure, I will rewrite this part to align the behavior with OVL driver.
Best, Paul-pl Chen
************* MEDIATEK Confidentiality Notice ******************** The information contained in this e-mail message (including any attachments) may be confidential, proprietary, privileged, or otherwise exempt from disclosure under applicable laws. It is intended to be conveyed only to the designated recipient(s). Any use, dissemination, distribution, printing, retaining or copying of this e-mail (including its attachments) by unintended recipient(s) is strictly prohibited and may be unlawful. If you are not an intended recipient of this e-mail, or believe that you have received this e-mail in error, please notify the sender immediately (by replying to this e-mail), delete any and all copies of this e-mail (including any attachments) from your system, and do not disclose the content of this e-mail to any other person. Thank you!
