On Fri, 09 May 2025, "Kandpal, Suraj" <suraj.kand...@intel.com> wrote: >> -----Original Message----- >> From: Intel-gfx <intel-gfx-boun...@lists.freedesktop.org> On Behalf Of >> gareth...@intel.com >> Sent: Friday, May 9, 2025 7:43 AM >> To: intel-gfx@lists.freedesktop.org; intel...@lists.freedesktop.org >> Cc: Yu, Gareth <gareth...@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...@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.kand...@intel.com> Cc : Jani Nikula >> <jani.nik...@intel.com> >> Signed-off-by: Gareth Yu <gareth...@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 ?
Fixes first, conversion to REG_BIT() and friends later. BR, 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 > -- Jani Nikula, Intel
