Re: [PATCH] drm: lcdif: Set and enable FIFO Panic threshold

[Marek Vasut <marex@denx.de>]

On 10/27/22 10:13, Marco Felsch 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),
>> +	       lcdif->base + LCDC_V8_PANIC0_THRES);
>> +
>> +	/*
>> +	 * Enable FIFO Panic, this does not generate interrupt, but
>> +	 * boosts NoC priority based on FIFO Panic watermarks.
>> +	 */
>> +	writel(INT_ENABLE_D1_PLANE_PANIC_EN,
>> +	       lcdif->base + LCDC_V8_INT_ENABLE_D1);
> 
> Out of curiosity since we have a patch doing the exact same thing but
> didn't saw any improvements. Is there a reason why you enabled it here?

It seems like the right thing to do here, when enabling the controller.

> We did this during lcdif_rpm_resume(). But as I said with a 1080P
> display we still saw the flickering, it disappeared first after rising
> the burst-size.

That's what the NXP downstream driver does too, isn't it ? That seems 
like the wrong place to me.