| Message ID | 0afc1d559c463fb5f9fc74b768df6a4e6bfcd2c6.1637588831.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: break intel_display_types.h dependency on i915_drv.h | expand |
On Mon, Nov 22, 2021 at 03:51:03PM +0200, Jani Nikula wrote: > There are only three call sites remaining for > intel_wait_for_vblank(). Remove the function, and open code it to avoid > new users from showing up. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/display/intel_cdclk.c | 2 +- > drivers/gpu/drm/i915/display/intel_crt.c | 2 +- > drivers/gpu/drm/i915/display/intel_display.c | 8 ++++++-- > drivers/gpu/drm/i915/display/intel_display_types.h | 8 -------- > 4 files changed, 8 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c > index 91c19e0a98d7..e3b863ee0bbb 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -1690,7 +1690,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > intel_de_write(dev_priv, CDCLK_CTL, val); > > if (pipe != INVALID_PIPE) > - intel_wait_for_vblank(dev_priv, pipe); > + drm_crtc_wait_one_vblank(&intel_get_crtc_for_pipe(dev_priv, pipe)->base); That looks rather hideuous. I think I'd prefer to keep the wrapper. > > if (DISPLAY_VER(dev_priv) >= 11) { > ret = sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, > diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c > index 8796527f74e5..43b3f6044f96 100644 > --- a/drivers/gpu/drm/i915/display/intel_crt.c > +++ b/drivers/gpu/drm/i915/display/intel_crt.c > @@ -721,7 +721,7 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe) > intel_uncore_posting_read(uncore, pipeconf_reg); > /* Wait for next Vblank to substitue > * border color for Color info */ > - intel_wait_for_vblank(dev_priv, pipe); > + drm_crtc_wait_one_vblank(&intel_get_crtc_for_pipe(dev_priv, pipe)->base); > st00 = intel_uncore_read8(uncore, _VGA_MSR_WRITE); > status = ((st00 & (1 << 4)) != 0) ? > connector_status_connected : > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 48d93d1f6c1a..1fc602bdfde1 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -2098,8 +2098,12 @@ static void hsw_crtc_enable(struct intel_atomic_state *state, > * to change the workaround. */ > hsw_workaround_pipe = new_crtc_state->hsw_workaround_pipe; > if (IS_HASWELL(dev_priv) && hsw_workaround_pipe != INVALID_PIPE) { > - intel_wait_for_vblank(dev_priv, hsw_workaround_pipe); > - intel_wait_for_vblank(dev_priv, hsw_workaround_pipe); > + struct intel_crtc *wa_crtc; > + > + wa_crtc = intel_get_crtc_for_pipe(dev_priv, hsw_workaround_pipe); > + > + drm_crtc_wait_one_vblank(&wa_crtc->base); > + drm_crtc_wait_one_vblank(&wa_crtc->base); > } > } > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > index a5508b8cdf63..2a18c4e554ef 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -2016,14 +2016,6 @@ intel_crtc_needs_modeset(const struct intel_crtc_state *crtc_state) > return drm_atomic_crtc_needs_modeset(&crtc_state->uapi); > } > > -static inline void > -intel_wait_for_vblank(struct drm_i915_private *dev_priv, enum pipe pipe) > -{ > - struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe); > - > - drm_crtc_wait_one_vblank(&crtc->base); > -} > - > static inline void > intel_wait_for_vblank_if_active(struct drm_i915_private *dev_priv, enum pipe pipe) > { > -- > 2.30.2
On Thu, Nov 25, 2021 at 12:23:22PM +0200, Ville Syrjälä wrote: > On Mon, Nov 22, 2021 at 03:51:03PM +0200, Jani Nikula wrote: > > There are only three call sites remaining for > > intel_wait_for_vblank(). Remove the function, and open code it to avoid > > new users from showing up. > > > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_cdclk.c | 2 +- > > drivers/gpu/drm/i915/display/intel_crt.c | 2 +- > > drivers/gpu/drm/i915/display/intel_display.c | 8 ++++++-- > > drivers/gpu/drm/i915/display/intel_display_types.h | 8 -------- > > 4 files changed, 8 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c > > index 91c19e0a98d7..e3b863ee0bbb 100644 > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > > @@ -1690,7 +1690,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > > intel_de_write(dev_priv, CDCLK_CTL, val); > > > > if (pipe != INVALID_PIPE) > > - intel_wait_for_vblank(dev_priv, pipe); > > + drm_crtc_wait_one_vblank(&intel_get_crtc_for_pipe(dev_priv, pipe)->base); > > That looks rather hideuous. I think I'd prefer to keep the wrapper. I guess if we had an intel_crtc based version of the vblank wait function it might not look so terrible. We could also s/intel_get_crtc_for_pipe/intel_crtc_for_pipe/ to make it a bit more succinct and look less like some refcounted thing.
On Thu, 25 Nov 2021, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Thu, Nov 25, 2021 at 12:23:22PM +0200, Ville Syrjälä wrote: >> On Mon, Nov 22, 2021 at 03:51:03PM +0200, Jani Nikula wrote: >> > There are only three call sites remaining for >> > intel_wait_for_vblank(). Remove the function, and open code it to avoid >> > new users from showing up. >> > >> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> > --- >> > drivers/gpu/drm/i915/display/intel_cdclk.c | 2 +- >> > drivers/gpu/drm/i915/display/intel_crt.c | 2 +- >> > drivers/gpu/drm/i915/display/intel_display.c | 8 ++++++-- >> > drivers/gpu/drm/i915/display/intel_display_types.h | 8 -------- >> > 4 files changed, 8 insertions(+), 12 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c >> > index 91c19e0a98d7..e3b863ee0bbb 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c >> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c >> > @@ -1690,7 +1690,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv, >> > intel_de_write(dev_priv, CDCLK_CTL, val); >> > >> > if (pipe != INVALID_PIPE) >> > - intel_wait_for_vblank(dev_priv, pipe); >> > + drm_crtc_wait_one_vblank(&intel_get_crtc_for_pipe(dev_priv, pipe)->base); >> >> That looks rather hideuous. I think I'd prefer to keep the wrapper. > > I guess if we had an intel_crtc based version of the vblank wait > function it might not look so terrible. That's what I had initially, actually, and was divided between the approaches. Ended up on this on the general principle of not adding local no-op wrappers. In any case, I think I do want to ditch the pipe-based wrapper because I don't want to encourage people to use that instead of the direct crtc version. If adding an intel_crtc based wrapper is good enough to make that happen, let's do that? > We could also s/intel_get_crtc_for_pipe/intel_crtc_for_pipe/ to make it > a bit more succinct and look less like some refcounted thing. Yeah, thought about that too but left it out of this series. BR, Jani.
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 91c19e0a98d7..e3b863ee0bbb 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -1690,7 +1690,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv, intel_de_write(dev_priv, CDCLK_CTL, val); if (pipe != INVALID_PIPE) - intel_wait_for_vblank(dev_priv, pipe); + drm_crtc_wait_one_vblank(&intel_get_crtc_for_pipe(dev_priv, pipe)->base); if (DISPLAY_VER(dev_priv) >= 11) { ret = sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c index 8796527f74e5..43b3f6044f96 100644 --- a/drivers/gpu/drm/i915/display/intel_crt.c +++ b/drivers/gpu/drm/i915/display/intel_crt.c @@ -721,7 +721,7 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe) intel_uncore_posting_read(uncore, pipeconf_reg); /* Wait for next Vblank to substitue * border color for Color info */ - intel_wait_for_vblank(dev_priv, pipe); + drm_crtc_wait_one_vblank(&intel_get_crtc_for_pipe(dev_priv, pipe)->base); st00 = intel_uncore_read8(uncore, _VGA_MSR_WRITE); status = ((st00 & (1 << 4)) != 0) ? connector_status_connected : diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 48d93d1f6c1a..1fc602bdfde1 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -2098,8 +2098,12 @@ static void hsw_crtc_enable(struct intel_atomic_state *state, * to change the workaround. */ hsw_workaround_pipe = new_crtc_state->hsw_workaround_pipe; if (IS_HASWELL(dev_priv) && hsw_workaround_pipe != INVALID_PIPE) { - intel_wait_for_vblank(dev_priv, hsw_workaround_pipe); - intel_wait_for_vblank(dev_priv, hsw_workaround_pipe); + struct intel_crtc *wa_crtc; + + wa_crtc = intel_get_crtc_for_pipe(dev_priv, hsw_workaround_pipe); + + drm_crtc_wait_one_vblank(&wa_crtc->base); + drm_crtc_wait_one_vblank(&wa_crtc->base); } } diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index a5508b8cdf63..2a18c4e554ef 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -2016,14 +2016,6 @@ intel_crtc_needs_modeset(const struct intel_crtc_state *crtc_state) return drm_atomic_crtc_needs_modeset(&crtc_state->uapi); } -static inline void -intel_wait_for_vblank(struct drm_i915_private *dev_priv, enum pipe pipe) -{ - struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe); - - drm_crtc_wait_one_vblank(&crtc->base); -} - static inline void intel_wait_for_vblank_if_active(struct drm_i915_private *dev_priv, enum pipe pipe) {
There are only three call sites remaining for intel_wait_for_vblank(). Remove the function, and open code it to avoid new users from showing up. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/display/intel_cdclk.c | 2 +- drivers/gpu/drm/i915/display/intel_crt.c | 2 +- drivers/gpu/drm/i915/display/intel_display.c | 8 ++++++-- drivers/gpu/drm/i915/display/intel_display_types.h | 8 -------- 4 files changed, 8 insertions(+), 12 deletions(-)