diff mbox series

[2/6] drm/i915: Change intel_pipe_update_{start, end}() calling convention

Message ID 20230828054140.28054-3-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: VRR and M/N stuff | expand

Commit Message

Ville Syrjala Aug. 28, 2023, 5:41 a.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We'll need to also look at the old crtc state in
intel_pipe_update_start() so change the calling convention to
just plumb in the full atomic state instead.

Cc: Manasi Navare <navaremanasi@chromium.org>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_crtc.c    | 18 ++++++++++++------
 drivers/gpu/drm/i915/display/intel_crtc.h    |  6 ++++--
 drivers/gpu/drm/i915/display/intel_display.c |  4 ++--
 3 files changed, 18 insertions(+), 10 deletions(-)

Comments

Manasi Navare Aug. 28, 2023, 6:30 p.m. UTC | #1 
On Sun, Aug 27, 2023 at 10:41 PM Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We'll need to also look at the old crtc state in
> intel_pipe_update_start() so change the calling convention to
> just plumb in the full atomic state instead.

I am guessing we would need the old crtc state to look at if VRR parameters
were changed?
Could we elaborate why we would need old crtc state so we better understand this
change in the patch?

Manasi

>
> Cc: Manasi Navare <navaremanasi@chromium.org>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_crtc.c    | 18 ++++++++++++------
>  drivers/gpu/drm/i915/display/intel_crtc.h    |  6 ++++--
>  drivers/gpu/drm/i915/display/intel_display.c |  4 ++--
>  3 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
> index 5caa928e5ce9..461949b48411 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> @@ -470,7 +470,8 @@ static int intel_mode_vblank_start(const struct drm_display_mode *mode)
>
>  /**
>   * intel_pipe_update_start() - start update of a set of display registers
> - * @new_crtc_state: the new crtc state
> + * @state: the atomic state
> + * @crtc: the crtc
>   *
>   * Mark the start of an update to pipe registers that should be updated
>   * atomically regarding vblank. If the next vblank will happens within
> @@ -480,10 +481,12 @@ static int intel_mode_vblank_start(const struct drm_display_mode *mode)
>   * until a subsequent call to intel_pipe_update_end(). That is done to
>   * avoid random delays.
>   */
> -void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state)
> +void intel_pipe_update_start(struct intel_atomic_state *state,
> +                            struct intel_crtc *crtc)
>  {
> -       struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
>         struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +       struct intel_crtc_state *new_crtc_state =
> +               intel_atomic_get_new_crtc_state(state, crtc);
>         const struct drm_display_mode *adjusted_mode = &new_crtc_state->hw.adjusted_mode;
>         long timeout = msecs_to_jiffies_timeout(1);
>         int scanline, min, max, vblank_start;
> @@ -631,15 +634,18 @@ static void dbg_vblank_evade(struct intel_crtc *crtc, ktime_t end) {}
>
>  /**
>   * intel_pipe_update_end() - end update of a set of display registers
> - * @new_crtc_state: the new crtc state
> + * @state: the atomic state
> + * @crtc: the crtc
>   *
>   * Mark the end of an update started with intel_pipe_update_start(). This
>   * re-enables interrupts and verifies the update was actually completed
>   * before a vblank.
>   */
> -void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
> +void intel_pipe_update_end(struct intel_atomic_state *state,
> +                          struct intel_crtc *crtc)
>  {
> -       struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
> +       struct intel_crtc_state *new_crtc_state =
> +               intel_atomic_get_new_crtc_state(state, crtc);
>         enum pipe pipe = crtc->pipe;
>         int scanline_end = intel_get_crtc_scanline(crtc);
>         u32 end_vbl_count = intel_crtc_get_vblank_counter(crtc);
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.h b/drivers/gpu/drm/i915/display/intel_crtc.h
> index 51a4c8df9e65..22d7993d1f0b 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.h
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.h
> @@ -36,8 +36,10 @@ void intel_crtc_state_reset(struct intel_crtc_state *crtc_state,
>  u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
>  void intel_crtc_vblank_on(const struct intel_crtc_state *crtc_state);
>  void intel_crtc_vblank_off(const struct intel_crtc_state *crtc_state);
> -void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state);
> -void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state);
> +void intel_pipe_update_start(struct intel_atomic_state *state,
> +                            struct intel_crtc *crtc);
> +void intel_pipe_update_end(struct intel_atomic_state *state,
> +                          struct intel_crtc *crtc);
>  void intel_wait_for_vblank_workers(struct intel_atomic_state *state);
>  struct intel_crtc *intel_first_crtc(struct drm_i915_private *i915);
>  struct intel_crtc *intel_crtc_for_pipe(struct drm_i915_private *i915,
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index f6397462e4c2..cfad967b5684 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6559,7 +6559,7 @@ static void intel_update_crtc(struct intel_atomic_state *state,
>         intel_crtc_planes_update_noarm(state, crtc);
>
>         /* Perform vblank evasion around commit operation */
> -       intel_pipe_update_start(new_crtc_state);
> +       intel_pipe_update_start(state, crtc);
>
>         commit_pipe_pre_planes(state, crtc);
>
> @@ -6567,7 +6567,7 @@ static void intel_update_crtc(struct intel_atomic_state *state,
>
>         commit_pipe_post_planes(state, crtc);
>
> -       intel_pipe_update_end(new_crtc_state);
> +       intel_pipe_update_end(state, crtc);
>
>         /*
>          * We usually enable FIFO underrun interrupts as part of the
> --
> 2.41.0
>
Ville Syrjala Aug. 29, 2023, 8:27 a.m. UTC | #2 
On Mon, Aug 28, 2023 at 11:30:25AM -0700, Manasi Navare wrote:
> On Sun, Aug 27, 2023 at 10:41 PM Ville Syrjala
> <ville.syrjala@linux.intel.com> wrote:
> >
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We'll need to also look at the old crtc state in
> > intel_pipe_update_start() so change the calling convention to
> > just plumb in the full atomic state instead.
> 
> I am guessing we would need the old crtc state to look at if VRR parameters
> were changed?
> Could we elaborate why we would need old crtc state so we better understand this
> change in the patch?

Details are in the patch that does those changes.

> 
> Manasi
> 
> >
> > Cc: Manasi Navare <navaremanasi@chromium.org>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_crtc.c    | 18 ++++++++++++------
> >  drivers/gpu/drm/i915/display/intel_crtc.h    |  6 ++++--
> >  drivers/gpu/drm/i915/display/intel_display.c |  4 ++--
> >  3 files changed, 18 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
> > index 5caa928e5ce9..461949b48411 100644
> > --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> > @@ -470,7 +470,8 @@ static int intel_mode_vblank_start(const struct drm_display_mode *mode)
> >
> >  /**
> >   * intel_pipe_update_start() - start update of a set of display registers
> > - * @new_crtc_state: the new crtc state
> > + * @state: the atomic state
> > + * @crtc: the crtc
> >   *
> >   * Mark the start of an update to pipe registers that should be updated
> >   * atomically regarding vblank. If the next vblank will happens within
> > @@ -480,10 +481,12 @@ static int intel_mode_vblank_start(const struct drm_display_mode *mode)
> >   * until a subsequent call to intel_pipe_update_end(). That is done to
> >   * avoid random delays.
> >   */
> > -void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state)
> > +void intel_pipe_update_start(struct intel_atomic_state *state,
> > +                            struct intel_crtc *crtc)
> >  {
> > -       struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
> >         struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > +       struct intel_crtc_state *new_crtc_state =
> > +               intel_atomic_get_new_crtc_state(state, crtc);
> >         const struct drm_display_mode *adjusted_mode = &new_crtc_state->hw.adjusted_mode;
> >         long timeout = msecs_to_jiffies_timeout(1);
> >         int scanline, min, max, vblank_start;
> > @@ -631,15 +634,18 @@ static void dbg_vblank_evade(struct intel_crtc *crtc, ktime_t end) {}
> >
> >  /**
> >   * intel_pipe_update_end() - end update of a set of display registers
> > - * @new_crtc_state: the new crtc state
> > + * @state: the atomic state
> > + * @crtc: the crtc
> >   *
> >   * Mark the end of an update started with intel_pipe_update_start(). This
> >   * re-enables interrupts and verifies the update was actually completed
> >   * before a vblank.
> >   */
> > -void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
> > +void intel_pipe_update_end(struct intel_atomic_state *state,
> > +                          struct intel_crtc *crtc)
> >  {
> > -       struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
> > +       struct intel_crtc_state *new_crtc_state =
> > +               intel_atomic_get_new_crtc_state(state, crtc);
> >         enum pipe pipe = crtc->pipe;
> >         int scanline_end = intel_get_crtc_scanline(crtc);
> >         u32 end_vbl_count = intel_crtc_get_vblank_counter(crtc);
> > diff --git a/drivers/gpu/drm/i915/display/intel_crtc.h b/drivers/gpu/drm/i915/display/intel_crtc.h
> > index 51a4c8df9e65..22d7993d1f0b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_crtc.h
> > +++ b/drivers/gpu/drm/i915/display/intel_crtc.h
> > @@ -36,8 +36,10 @@ void intel_crtc_state_reset(struct intel_crtc_state *crtc_state,
> >  u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
> >  void intel_crtc_vblank_on(const struct intel_crtc_state *crtc_state);
> >  void intel_crtc_vblank_off(const struct intel_crtc_state *crtc_state);
> > -void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state);
> > -void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state);
> > +void intel_pipe_update_start(struct intel_atomic_state *state,
> > +                            struct intel_crtc *crtc);
> > +void intel_pipe_update_end(struct intel_atomic_state *state,
> > +                          struct intel_crtc *crtc);
> >  void intel_wait_for_vblank_workers(struct intel_atomic_state *state);
> >  struct intel_crtc *intel_first_crtc(struct drm_i915_private *i915);
> >  struct intel_crtc *intel_crtc_for_pipe(struct drm_i915_private *i915,
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index f6397462e4c2..cfad967b5684 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -6559,7 +6559,7 @@ static void intel_update_crtc(struct intel_atomic_state *state,
> >         intel_crtc_planes_update_noarm(state, crtc);
> >
> >         /* Perform vblank evasion around commit operation */
> > -       intel_pipe_update_start(new_crtc_state);
> > +       intel_pipe_update_start(state, crtc);
> >
> >         commit_pipe_pre_planes(state, crtc);
> >
> > @@ -6567,7 +6567,7 @@ static void intel_update_crtc(struct intel_atomic_state *state,
> >
> >         commit_pipe_post_planes(state, crtc);
> >
> > -       intel_pipe_update_end(new_crtc_state);
> > +       intel_pipe_update_end(state, crtc);
> >
> >         /*
> >          * We usually enable FIFO underrun interrupts as part of the
> > --
> > 2.41.0
> >
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
index 5caa928e5ce9..461949b48411 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -470,7 +470,8 @@  static int intel_mode_vblank_start(const struct drm_display_mode *mode)
 
 /**
  * intel_pipe_update_start() - start update of a set of display registers
- * @new_crtc_state: the new crtc state
+ * @state: the atomic state
+ * @crtc: the crtc
  *
  * Mark the start of an update to pipe registers that should be updated
  * atomically regarding vblank. If the next vblank will happens within
@@ -480,10 +481,12 @@  static int intel_mode_vblank_start(const struct drm_display_mode *mode)
  * until a subsequent call to intel_pipe_update_end(). That is done to
  * avoid random delays.
  */
-void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state)
+void intel_pipe_update_start(struct intel_atomic_state *state,
+			     struct intel_crtc *crtc)
 {
-	struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_crtc_state *new_crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
 	const struct drm_display_mode *adjusted_mode = &new_crtc_state->hw.adjusted_mode;
 	long timeout = msecs_to_jiffies_timeout(1);
 	int scanline, min, max, vblank_start;
@@ -631,15 +634,18 @@  static void dbg_vblank_evade(struct intel_crtc *crtc, ktime_t end) {}
 
 /**
  * intel_pipe_update_end() - end update of a set of display registers
- * @new_crtc_state: the new crtc state
+ * @state: the atomic state
+ * @crtc: the crtc
  *
  * Mark the end of an update started with intel_pipe_update_start(). This
  * re-enables interrupts and verifies the update was actually completed
  * before a vblank.
  */
-void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
+void intel_pipe_update_end(struct intel_atomic_state *state,
+			   struct intel_crtc *crtc)
 {
-	struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
+	struct intel_crtc_state *new_crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
 	enum pipe pipe = crtc->pipe;
 	int scanline_end = intel_get_crtc_scanline(crtc);
 	u32 end_vbl_count = intel_crtc_get_vblank_counter(crtc);
diff --git a/drivers/gpu/drm/i915/display/intel_crtc.h b/drivers/gpu/drm/i915/display/intel_crtc.h
index 51a4c8df9e65..22d7993d1f0b 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.h
+++ b/drivers/gpu/drm/i915/display/intel_crtc.h
@@ -36,8 +36,10 @@  void intel_crtc_state_reset(struct intel_crtc_state *crtc_state,
 u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
 void intel_crtc_vblank_on(const struct intel_crtc_state *crtc_state);
 void intel_crtc_vblank_off(const struct intel_crtc_state *crtc_state);
-void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state);
-void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state);
+void intel_pipe_update_start(struct intel_atomic_state *state,
+			     struct intel_crtc *crtc);
+void intel_pipe_update_end(struct intel_atomic_state *state,
+			   struct intel_crtc *crtc);
 void intel_wait_for_vblank_workers(struct intel_atomic_state *state);
 struct intel_crtc *intel_first_crtc(struct drm_i915_private *i915);
 struct intel_crtc *intel_crtc_for_pipe(struct drm_i915_private *i915,
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index f6397462e4c2..cfad967b5684 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6559,7 +6559,7 @@  static void intel_update_crtc(struct intel_atomic_state *state,
 	intel_crtc_planes_update_noarm(state, crtc);
 
 	/* Perform vblank evasion around commit operation */
-	intel_pipe_update_start(new_crtc_state);
+	intel_pipe_update_start(state, crtc);
 
 	commit_pipe_pre_planes(state, crtc);
 
@@ -6567,7 +6567,7 @@  static void intel_update_crtc(struct intel_atomic_state *state,
 
 	commit_pipe_post_planes(state, crtc);
 
-	intel_pipe_update_end(new_crtc_state);
+	intel_pipe_update_end(state, crtc);
 
 	/*
 	 * We usually enable FIFO underrun interrupts as part of the