> -----Original Message-----
> From: Kandpal, Suraj <[email protected]>
> Sent: Friday, May 9, 2025 12:17 PM
> To: Yu, Gareth <[email protected]>; [email protected]; intel-
> [email protected]
> Cc: Yu, Gareth <[email protected]>; Nikula, Jani <[email protected]>
> Subject: RE: [PATCH] drm/i915/display: Send DSI DCS commands with pipeline
> flush in High Speed
>
>
>
> > -----Original Message-----
> > From: Intel-gfx <[email protected]> On Behalf Of
> > [email protected]
> > Sent: Friday, May 9, 2025 7:43 AM
> > To: [email protected]; [email protected]
> > Cc: Yu, Gareth <[email protected]>
> > Subject: [PATCH] drm/i915/display: Send DSI DCS commands with pipeline
> > flush in High Speed
> >
>
> Clip the subject make it a little smaller
How about "Always send high speed commands with pipeline flush"?
>
> > From: Gareth Yu <[email protected]>
> >
> > 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
BSPEC: 50193
>
>
> >
> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14247
> >
> > Cc : Suraj Kandpal <[email protected]> Cc : Jani Nikula
> > <[email protected]>
> > Signed-off-by: Gareth Yu <[email protected]>
> > ---
> > 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
