Re: [PATCH] drm/tidss: Update Videoport SYNC LOST IRQ bit

[Devarsh Thakkar]

Hi Hari,

On 03/09/25 15:39, Harikrishna Shenoy wrote:
> Update VP SYNC LOST Bit as per register description for
> DSS0_COMMON_VP_IRQENABLE_0 give in TRM.
>
> Link:https://www.ti.com/lit/zip/spruil1/SPRUIL_DRA829_TDA4VM
> Table 12-597. DSS0_COMMON_VP_IRQENABLE_0
>
> Fixes: 32a1795f57ee ("drm/tidss: New driver for TI Keystone platform Display SubSystem")
>
> Signed-off-by: Harikrishna Shenoy <h-shenoy@xxxxxx>
> ---
>   drivers/gpu/drm/tidss/tidss_irq.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tidss/tidss_irq.h b/drivers/gpu/drm/tidss/tidss_irq.h
> index dd61f645f662..0194010a7fff 100644
> --- a/drivers/gpu/drm/tidss/tidss_irq.h
> +++ b/drivers/gpu/drm/tidss/tidss_irq.h
> @@ -53,7 +53,7 @@ static inline dispc_irq_t DSS_IRQ_PLANE_MASK(u32 plane)
>   #define DSS_IRQ_VP_FRAME_DONE(ch)	DSS_IRQ_VP_BIT((ch), 0)
>   #define DSS_IRQ_VP_VSYNC_EVEN(ch)	DSS_IRQ_VP_BIT((ch), 1)
>   #define DSS_IRQ_VP_VSYNC_ODD(ch)	DSS_IRQ_VP_BIT((ch), 2)
> -#define DSS_IRQ_VP_SYNC_LOST(ch)	DSS_IRQ_VP_BIT((ch), 3)
> +#define DSS_IRQ_VP_SYNC_LOST(ch)	DSS_IRQ_VP_BIT((ch), 4)

I think DSS_IRQ_VP_BIT((ch), 3) is correct only. As we discussed, above 
bits correspond to virtual mask that driver is using as mentioned in the 
comment :

/
 * 

 * bit group |dev|wb |mrg0|mrg1|mrg2|mrg3|plane0-3| <unused> | 

 * bit use   |D  |fou|FEOL|FEOL|FEOL|FEOL|  UUUU  |          | 

 * bit number|0  |1-3|4-7 |8-11|  12-19  | 20-23  |  24-31   | 

 *
/

Furthermore, it gives correct value per the above table when sync lost 
occurs :
[ 2245.588346] tidss 30220000.dss: CRTC1 SYNC LOST: (irq 800)

Hence, NACK for this patch.

Regards
Devarsh











>   
>   #define DSS_IRQ_PLANE_FIFO_UNDERFLOW(plane)	DSS_IRQ_PLANE_BIT((plane), 0)
>