On Thu, Mar 28, 2024 at 09:16:06AM +0000, Murthy, Arun R wrote: > > > -----Original Message----- > > From: Intel-gfx <[email protected]> On Behalf Of Ville > > Syrjala > > Sent: Wednesday, March 27, 2024 11:16 PM > > To: [email protected] > > Subject: [PATCH 01/13] drm/i915/cdclk: Fix CDCLK programming order when > > pipes are active > > > > From: Ville Syrjälä <[email protected]> > > > > Currently we always reprogram CDCLK from the > > intel_set_cdclk_pre_plane_update() when using squahs/crawl. > Typo squashs->squash > > > The code only works correctly for the cd2x update or full modeset cases, > > and it > > was simply never updated to deal with squash/crawl. > > > > If the CDCLK frequency is increasing we must reprogram it before we do > > anything else that might depend on the new higher frequency, and conversely > > we must not decrease the frequency until everything that might still depend > > on > > the old higher frequency has been dealt with. > > > > Since cdclk_state->pipe is only relevant when doing a cd2x update we can't > > use > > it to determine the correct sequence during squash/crawl. To that end > > introduce cdclk_state->disable_pipes which simply indicates that we must > > perform the update while the pipes are disable (ie. during > > intel_set_cdclk_pre_plane_update()). Otherwise we use the same old vs. new > > CDCLK frequency comparsiong as for cd2x updates. > > > > The only remaining problem case is when the voltage_level needs to increase > > due to a DDI port, but the CDCLK frequency is decreasing (and not all pipes > > are > > being disabled). The current approach will not bump the voltage level up > > until > > after the port has already been enabled, which is too late. > > But we'll take care of that case separately. > > > > v2: Don't break the "must disable pipes case" > > > > Signed-off-by: Ville Syrjälä <[email protected]> > > --- > > drivers/gpu/drm/i915/display/intel_cdclk.c | 15 +++++++++------ > > drivers/gpu/drm/i915/display/intel_cdclk.h | 3 +++ > > 2 files changed, 12 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > > b/drivers/gpu/drm/i915/display/intel_cdclk.c > > index 31aaa9780dfc..619529dba095 100644 > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > > @@ -2600,7 +2600,6 @@ intel_set_cdclk_pre_plane_update(struct > > intel_atomic_state *state) > > intel_atomic_get_old_cdclk_state(state); > > const struct intel_cdclk_state *new_cdclk_state = > > intel_atomic_get_new_cdclk_state(state); > > - enum pipe pipe = new_cdclk_state->pipe; > Looks like this cdclk_state->pipe is not more used in the driver and can it > be removed?
It is still used for its primary purpose (cd2x update pipe select). The only thing changing here is that we no longer use it as a canary to indicate whether we need to do the cdclk programming with pipes off or not. -- Ville Syrjälä Intel
