<html><body><p>
<pre>
Hi CK,

On Wed, 2024-06-12 at 01:35 +0000, CK Hu (胡俊光) wrote:
> Hi, Shawn:
>
> On Thu, 2024-06-06 at 17:26 +0800, Shawn Sung wrote:
> > From: Hsiao Chien Sung <shawn.sung@mediatek.com>
> >
> > We choose OVL as the CRC generator from other hardware
> > components that are also capable of calculating CRCs,
> > since its frame done event triggers vblanks, it can be
> > used as a signal to know when is safe to retrieve CRC of
> > the frame.
> >
> > Please note that position of the hardware component
> > that is chosen as CRC generator in the display path is
> > significant. For example, while OVL is the first module
> > in VDOSYS0, its CRC won't be affected by the modules
> > after it, which means effects applied by PQ, Gamma,
> > Dither or any other components after OVL won't be
> > calculated in CRC generation.
> >
> > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> > ---
>
> [snip]
>
> >
> > void mtk_ovl_stop(struct device *dev)
> > @@ -489,6 +579,83 @@ 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;
> >
> > +/*
> > + * OVL only supports 8 bits data in CRC calculation, transform
> > 10-bit
> > + * RGB to 8-bit RGB by leveraging the ability of the Y2R (YUV-
> > to-RGB)
> > + * hardware to multiply coefficients, although there is nothing
> > to do
> > + * with the YUV format.
> > + */
> > +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 coefficient 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;
>
> I don't know why do this? If an input value is 0x100, then
>
> 0x100 right shit 2 bit become 0x40.
> 0x40 - 1 = 0x3f.
> 0x3f left shift 2 bit become 0xfc.
>
> So input 0x100 and output 0xfc. Why?
>
This should be -2, not -1.
It is to rid of the last 2 bit.

OVL HW does not use >> 2 and then << 2, but uses *0.25 and then rounds
to a positive integer and then *4, so it is necessary to remove the
last 2 bits by -2 before doing the operation.

If the formula is: round(input data *0.25, 0) *4
The input data is 3, and it will become 4 after calculation.

If the formula is: round((input data - 2) *0.25, 0) *4
The input data is 3, and it will become 0 after calculation.

> > +
> > +/*
> > + * R2R coefficient 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
>
> You truncate the last two bit, so some quality lost. I think the
> quality is main function and CRC is just for debug. So it's better
> that
> in normal case we keep quality and only for debug to lost the
> quality.

I agree with this, I'll add crc.opend flag to bypass these settings.

>
> I have another question. You just truncate the last two bit but it is
> still 10 bit value, so CRC could calculate this 10 bit value? I don't
> know why you say CRC just for 8 bit?
>

OVL is 10-bit per RGB channel, so the 8-bit value will be padding with
2-bits 0 in the end, so the CRC truncating the last 2-bit in 10-bit
value would be the same as original case.

> Regards,
> CK
>
> > + * [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;
> >

</pre>
</p></body></html><!--type:text--><!--{--><pre>************* 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!
</pre><!--}-->