Re: [PATCH v8, 5/6] drm/mediatek: add RDMA fifo size error handle

[Chun-Kuang Hu]

Hi, Yongqiang:

Yongqiang Niu <yongqiang.niu@xxxxxxxxxxxx> 於 2021年1月5日 週二 下午2:36寫道：
>
> On Mon, 2020-12-14 at 22:54 +0800, Chun-Kuang Hu wrote:
> > Hi, Yongqiang:
> >
> > Yongqiang Niu <yongqiang.niu@xxxxxxxxxxxx> 於 2020年12月11日 週五 上午8:45寫道：
> > >
> > > On Thu, 2020-12-10 at 23:50 +0800, Chun-Kuang Hu wrote:
> > > > Hi, Yongqiang:
> > > >
> > > > Yongqiang Niu <yongqiang.niu@xxxxxxxxxxxx> 於 2020年12月10日 週四 下午5:08寫道：
> > > > >
> > > > > This patch add RDMA fifo size error handle
> > > > > rdma fifo size will not always bigger than the calculated threshold
> > > > > if that case happened, we need set fifo size as the threshold
> > > > >
> > > > > Signed-off-by: Yongqiang Niu <yongqiang.niu@xxxxxxxxxxxx>
> > > > > ---
> > > > >  drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 4 ++++
> > > > >  1 file changed, 4 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > > > > index 794acc5..0508392 100644
> > > > > --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > > > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > > > > @@ -151,6 +151,10 @@ static void mtk_rdma_config(struct mtk_ddp_comp *comp, unsigned int width,
> > > > >          * account for blanking, and with a pixel depth of 4 bytes:
> > > > >          */
> > > > >         threshold = width * height * vrefresh * 4 * 7 / 1000000;
> > > > > +
> > > > > +       if (threshold > rdma_fifo_size)
> > > > > +               threshold = rdma_fifo_size;
> > > >
> > > > If the formula is not correct, you should fix the formula not work around.
> > > >
> > > > Regards,
> > > > Chun-Kuang.
> > >
> > > how about this:
> > > threshold = max(width * height * vrefresh * 4 * 7 / 1000000,
> > > rdma_fifo_size);
> >
> > When I use width = 1920, height = 1080, vrefresh = 60 to calculate, I
> > get threshold = 2985.
> > So I think set threshold to half of fifo size is OK for MAX_WIDTH,
> > MAX_HEIGHT, MAX_VREFRESH (these three may be different in each SoC)
> >
> > threshold = RDMA_FIFO_PSEUDO_SIZE(rdma_fifo_size) / 2;
> >
> > But I worry half fifo size is too big for small resolution and let
> > small resolution too easy to trigger burst read DRAM. So let the
> > formula to be this:
> >
> > threshold = RDMA_FIFO_PSEUDO_SIZE(rdma_fifo_size) * width * height *
> > vrefresh / 2 / MAX_WIDTH / MAX_HEIGHT / MAX_VREFRESH;
> >
> > How do you think about this?
> >
> > Regards,
> > Chun-Kuang.
>
> how about remove this formula, and set threshold = rdma_fifo_size

This means that RDMA would always trigger burst read. In normal case,
I think no hardware would trigger burst read. Only the emergency
occur, the hardware trigger burst read. RDMA always triggering burst
read is good for RDMA, but it may hurt other hardware to read DRAM. So
I think RDMA should trigger burst read only when fifo data is under a
emergency threshold.

Regards,
Chun-Kuang.

> >
> > > >
> > > > > +
> > > > >         reg = RDMA_FIFO_UNDERFLOW_EN |
> > > > >               RDMA_FIFO_PSEUDO_SIZE(rdma_fifo_size) |
> > > > >               RDMA_OUTPUT_VALID_FIFO_THRESHOLD(threshold);
> > > > > --
> > > > > 1.8.1.1.dirty
> > > > > _______________________________________________
> > > > > Linux-mediatek mailing list
> > > > > Linux-mediatek@xxxxxxxxxxxxxxxxxxx
> > > > > http://lists.infradead.org/mailman/listinfo/linux-mediatek
> > >
>