On 22-10-27, Liu Ying wrote:
> On Thu, 2022-10-27 at 12:03 +0200, Marek Vasut wrote:
> > On 10/27/22 07:45, Liu Ying wrote:
> >
> > Hi,
> >
> > [...]
> >
> > > > diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c
> > > > b/drivers/gpu/drm/mxsfb/lcdif_kms.c
> > > > index a5302006c02cd..aee7babb5fa5c 100644
> > > > --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
> > > > +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
> > > > @@ -341,6 +341,18 @@ static void lcdif_enable_controller(struct
> > > > lcdif_drm_private *lcdif)
> > > > reg = readl(lcdif->base + LCDC_V8_CTRLDESCL0_5);
> > > > reg |= CTRLDESCL0_5_EN;
> > > > writel(reg, lcdif->base + LCDC_V8_CTRLDESCL0_5);
> > > > +
> > > > + /* Set FIFO Panic watermarks, low 1/3, high 2/3 . */
> > > > + writel(FIELD_PREP(PANIC0_THRES_LOW_MASK, 1 * PANIC0_THRES_RANGE
> > > > / 3) |
> > > > + FIELD_PREP(PANIC0_THRES_HIGH_MASK, 2 *
> > > > PANIC0_THRES_RANGE / 3),
> > >
> > > Better to define PANIC0_THRES_{LOW,HIGH}(n) macros in lcdif_regs.h?
> > >
> > > Downstream kernel uses the below threshold values:
> > > a) i.MX8mp EVK board with LPDDR4
> > > 1/3 and 2/3 for LCDIF{1,2} + DSI/LVDS - default values in driver
> > > 1/2 and 3/4 for LCDIF3 + HDMI - set in device tree
> > >
> > > b) i.MX8mp EVK board with DDR4
> > > 1/3 and 2/3 for LCDIF{1,2} + DSI/LVDS - default values in driver
> > > 2/3 and 3/3 for LCDIF3 + HDMI - set in devic tree
> > >
> > > Jian told me that LCDIF3 needs different sets of threshold values
> > > for
> > > different types of DDR to avoid 4k HDMI display issues and the
> > > threshold values impact overall DDR/bus utilization(?), so
> > > downstream
> > > kernel chooses to get optional threshold value properties from
> > > LCDIF DT
> > > node.
> > >
> > > Instead of always using 1/3 and 2/3, maybe there are three options:
> > > 1) Same to downstream kernel, take 1/3 and 2/3 as default values
> > > and
> > > get optional threshold values from DT properties - no additional
> > > properties are acceptable in the existing DT binding doc?
> > > 2) Check pixel clock rate, and if it is greater than a certain
> > > value,
> > > use 2/3 and 3/3. Otherwise, use 1/3 and 2/3.
> > > 3) Always use 2/3 and 3/3.
> >
> > Why 2/3 and 3/3 instead of 1/3 and 2/3 ?
>
> 2/3 and 3/3 trigger panic signal more easily than 1/3 and 2/3.
>
> >
> > Seems like 1/3 and 2/3 provides enough FIFO margin for every
> > scenario.
>
> I didn't tune the threshold values. What I was told is that some
> usecases suffer from the FIFO underflows with 1/3 and 2/3. And, it
> appears that FIFO doesn't underflow with 1/2 and 3/4 or 2/3 and 3/3 in
> those usecases. That's why downstream kernel chooses to use 1/2 and
> 3/4 or 2/3 and 3/3.
Hi Liu Marek,
I thought that: If the PANIC is enabled and the pre-configured
panic-priority is high enough, nothing should interrupt the LCDIF in
panic mode since it has the highest prio? So why does it the downstream
kernel configure it differently for different use-cases?
Also IMHO the threshold should be taken wisely to not enter panic mode
to early to not block others from the bus e.g. the GPU.
Regards,
Marco
