[Intel-gfx] [V13 4/5] drm/i915/dsi: Initiate fame request in cmd mode

[Kulkarni, Vandita]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Sent: Wednesday, September 23, 2020 4:03 PM
> To: Kulkarni, Vandita <vandita.kulkarni at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; Nikula, Jani <jani.nikula at intel.com>
> Subject: Re: [V13 4/5] drm/i915/dsi: Initiate fame request in cmd mode
> 
> On Wed, Sep 23, 2020 at 10:02:49AM +0000, Kulkarni, Vandita wrote:
> > > -----Original Message-----
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > Sent: Wednesday, September 23, 2020 3:30 PM
> > > To: Kulkarni, Vandita <vandita.kulkarni at intel.com>
> > > Cc: intel-gfx at lists.freedesktop.org; Nikula, Jani
> > > <jani.nikula at intel.com>
> > > Subject: Re: [V13 4/5] drm/i915/dsi: Initiate fame request in cmd
> > > mode
> > >
> > > On Tue, Sep 22, 2020 at 07:14:25PM +0530, Vandita Kulkarni wrote:
> > > > In TE Gate mode or TE NO_GATE mode on every flip we need to set
> > > > the frame update request bit.
> > > > After this  bit is set transcoder hardware will automatically send
> > > > the frame data to the panel in case of TE NO_GATE mode, where it
> > > > sends after it receives the TE event in case of TE_GATE mode.
> > > > Once the frame data is sent to the panel, we see the frame counter
> > > > updating.
> > > >
> > > > v2: Use intel_de_read/write
> > > >
> > > > v3: remove the usage of private_flags
> > > >
> > > > v4: Use icl_dsi in func names if non static,
> > > >     fix code formatting issues. (Jani)
> > > >
> > > > Signed-off-by: Vandita Kulkarni <vandita.kulkarni at intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/icl_dsi.c       | 26
> ++++++++++++++++++++
> > > >  drivers/gpu/drm/i915/display/intel_display.c | 10 ++++++++
> > > >  drivers/gpu/drm/i915/display/intel_dsi.h     |  1 +
> > > >  3 files changed, 37 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> > > > b/drivers/gpu/drm/i915/display/icl_dsi.c
> > > > index 2789020e20db..7d2abc7f6ba3 100644
> > > > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> > > > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> > > > @@ -205,6 +205,32 @@ static int dsi_send_pkt_payld(struct
> > > > intel_dsi_host
> > > *host,
> > > >  	return 0;
> > > >  }
> > > >
> > > > +void icl_dsi_frame_update(struct intel_crtc_state *crtc_state) {
> > > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > > +	u32 tmp, flags;
> > > > +	enum port port;
> > > > +
> > > > +	flags = crtc->mode_flags;
> > > > +
> > > > +	/*
> > > > +	 * case 1 also covers dual link
> > > > +	 * In case of dual link, frame update should be set on
> > > > +	 * DSI_0
> > > > +	 */
> > > > +	if (flags & I915_MODE_FLAG_DSI_USE_TE0)
> > > > +		port = PORT_A;
> > > > +	else if (flags & I915_MODE_FLAG_DSI_USE_TE1)
> > > > +		port = PORT_B;
> > > > +	else
> > > > +		return;
> > > > +
> > > > +	tmp = intel_de_read(dev_priv, DSI_CMD_FRMCTL(port));
> > > > +	tmp |= DSI_FRAME_UPDATE_REQUEST;
> > > > +	intel_de_write(dev_priv, DSI_CMD_FRMCTL(port), tmp); }
> > > > +
> > > >  static void dsi_program_swing_and_deemphasis(struct intel_encoder
> > > > *encoder)  {
> > > >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > diff
> > > > --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index 5a9d933e425a..c4f331f2af45 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -15616,6 +15616,16 @@ static void
> > > > intel_atomic_commit_tail(struct
> > > intel_atomic_state *state)
> > > >  		intel_set_cdclk_post_plane_update(state);
> > > >  	}
> > > >
> > > > +	/*
> > > > +	 * Incase of mipi dsi command mode, we need to set frame update
> > > > +	 * for every commit
> > > > +	 */
> > > > +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i)
> > > > +		if (INTEL_GEN(dev_priv) >= 11 &&
> > > > +		    intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DSI))
> > > > +			if (new_crtc_state->hw.active)
> > > > +				icl_dsi_frame_update(new_crtc_state);
> > > > +
> > >
> > > Still the wrong place.
> > Should I be adding it at the end of pipe update? As we need TE to be
> enabled when we send frame update.
> 
> If it needs te then it should probably enable te.

Thanks for pointing this out.
Just to make sure on my assumption of keeping TE enabled before sending frame update,
I cross checked with the hw team and there seems to be no such rule.
Frame update request will be valid until the transcoder starts the frame.
Like you pointed out will add this to the beginning of pipe_update_end,
And send the next version.

Thanks,
Vandita


> 
> --
> Ville Syrjälä
> Intel