[PATCH] drm/i915/display: Send DSI DCS commands with pipeline flush in High Speed

[Kandpal, Suraj]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of
> gareth.yu at intel.com
> Sent: Friday, May 9, 2025 7:43 AM
> To: intel-gfx at lists.freedesktop.org; intel-xe at lists.freedesktop.org
> Cc: Yu, Gareth <gareth.yu at intel.com>
> Subject: [PATCH] drm/i915/display: Send DSI DCS commands with pipeline
> flush in High Speed
> 

Clip the subject make it a little smaller

> From: Gareth Yu <gareth.yu at intel.com>
> 
> With all of the boundary conditions when streaming the commands B2B in our
> validation (part of the reason we added the flush),  the Flush effectively
> serializes the transmission of each command enqueued within the command
> dispatcher to one per V. Blank line which simplifies the behavior of the High
> Speed Arbitration.
> 
> So, unless we absolutely have to burst these to the Sink, we should be using
> the Pipeline Flush bit to serialize the commands.

Do we have a bspec reference and/or dsi spec reference through which we can
verify the data that needs to be filled out for the dsi packet header.
Add it here will help make the review smoother and easier


> 
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14247
> 
> Cc : Suraj Kandpal <suraj.kandpal at intel.com> Cc : Jani Nikula
> <jani.nikula at intel.com>
> Signed-off-by: Gareth Yu <gareth.yu at intel.com>
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c      | 4 ++--
>  drivers/gpu/drm/i915/display/icl_dsi_regs.h | 1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 402b7b2e1829..f91f27067768 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -191,12 +191,12 @@ static int dsi_send_pkt_hdr(struct intel_dsi_host
> *host,
>  	else
>  		tmp &= ~PAYLOAD_PRESENT;
> 
> -	tmp &= ~VBLANK_FENCE;
> +	tmp &= ~(VBLANK_FENCE | LP_DATA_TRANSFER | PIPELINE_FLUSH);
> 
>  	if (enable_lpdt)
>  		tmp |= LP_DATA_TRANSFER;
>  	else
> -		tmp &= ~LP_DATA_TRANSFER;
> +		tmp |= PIPELINE_FLUSH;

Seems a little fishy but cannot be sure till I have the spec and I can verify it

> 
>  	tmp &= ~(PARAM_WC_MASK | VC_MASK | DT_MASK);
>  	tmp |= ((packet->header[0] & VC_MASK) << VC_SHIFT); diff --git
> a/drivers/gpu/drm/i915/display/icl_dsi_regs.h
> b/drivers/gpu/drm/i915/display/icl_dsi_regs.h
> index d4845ac65acc..b601b7632339 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi_regs.h
> +++ b/drivers/gpu/drm/i915/display/icl_dsi_regs.h
> @@ -272,6 +272,7 @@
>  #define  PAYLOAD_PRESENT		(1 << 31)
>  #define  LP_DATA_TRANSFER		(1 << 30)
>  #define  VBLANK_FENCE			(1 << 29)
> +#define  PIPELINE_FLUSH			(1 << 28)

We now follow the REG_BIT(28) macro instead of using  (1 << 28) convention
Would like to see that change too, ill send over a fix to get this to file use REG_BIT instead.
Or do we leave it as is ? What do you think Jani ?

Regards,
Suraj Kandpal 

>  #define  PARAM_WC_MASK			(0xffff << 8)
>  #define  PARAM_WC_LOWER_SHIFT		8
>  #define  PARAM_WC_UPPER_SHIFT		16
> --
> 2.34.1