Re: [Intel-gfx] [PATCH v2 07/11] drm/i915: Add a atomic evasion step to watermark programming, v2.
On Wed, Oct 26, 2016 at 07:19:45PM +0300, Ville Syrjälä wrote: > On Wed, Oct 26, 2016 at 03:41:35PM +0200, Maarten Lankhorst wrote: > > Allow the driver to write watermarks during atomic evasion. > > This will make it possible to write the watermarks in a cleaner > > way on gen9+. > > > > intel_atomic_state is not used here yet, but will be used when > > we program all watermarks as a separate step during evasion. > > > > This also writes linetime all the time, while before it was only > > done during plane updates. This looks like this could be a bugfix, > > but I'm not sure what it affects. > > > > Changes since v1: > > - Add comment about atomic evasion to commit message. > > - Unwrap I915_WRITE call. (Lyude) > > > > Signed-off-by: Maarten Lankhorst > > Cc: Matt Roper > > Cc: Paulo Zanoni > > --- > > drivers/gpu/drm/i915/i915_drv.h | 6 -- > > drivers/gpu/drm/i915/intel_display.c | 20 +--- > > drivers/gpu/drm/i915/intel_pm.c | 18 -- > > 3 files changed, 29 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 7a621c74254e..7a477d6a486e 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -485,6 +485,7 @@ struct sdvo_device_mapping { > > > > struct intel_connector; > > struct intel_encoder; > > +struct intel_atomic_state; > > struct intel_crtc_state; > > struct intel_initial_plane_config; > > struct intel_crtc; > > @@ -498,8 +499,9 @@ struct drm_i915_display_funcs { > > int (*compute_intermediate_wm)(struct drm_device *dev, > >struct intel_crtc *intel_crtc, > >struct intel_crtc_state *newstate); > > - void (*initial_watermarks)(struct intel_crtc_state *cstate); > > - void (*optimize_watermarks)(struct intel_crtc_state *cstate); > > + void (*initial_watermarks)(struct intel_atomic_state *state, struct > > intel_crtc_state *cstate); > > + void (*atomic_evade_watermarks)(struct intel_atomic_state *state, > > struct intel_crtc_state *cstate); > > Same drive by comment I gave before somewhere: > That name is still super confusing. We're not trying to evade watermarks are > we? The atomic_update_watermarks suggestion I've seen fly by on irc sounded good. Aside: some proper kerneldocs for display funcs would be also really good ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 07/11] drm/i915: Add a atomic evasion step to watermark programming, v2.
Minor grammar fix in the headline...it should be "an atomic" rather than "a atomic." Sorry I missed that the first time around. I know i915 tends to be somewhat lax about adhering to the 80-character line limit, but the lines that add the extra parameter are starting to get _really_ long now, so it would be good to wrap them if possible. Otherwise the change here look good to me, so with the minor cosmetic stuff fixed, Reviewed-by: Matt Roper Matt On Wed, Oct 26, 2016 at 03:41:35PM +0200, Maarten Lankhorst wrote: > Allow the driver to write watermarks during atomic evasion. > This will make it possible to write the watermarks in a cleaner > way on gen9+. > > intel_atomic_state is not used here yet, but will be used when > we program all watermarks as a separate step during evasion. > > This also writes linetime all the time, while before it was only > done during plane updates. This looks like this could be a bugfix, > but I'm not sure what it affects. > > Changes since v1: > - Add comment about atomic evasion to commit message. > - Unwrap I915_WRITE call. (Lyude) > > Signed-off-by: Maarten Lankhorst > Cc: Matt Roper > Cc: Paulo Zanoni > --- > drivers/gpu/drm/i915/i915_drv.h | 6 -- > drivers/gpu/drm/i915/intel_display.c | 20 +--- > drivers/gpu/drm/i915/intel_pm.c | 18 -- > 3 files changed, 29 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 7a621c74254e..7a477d6a486e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -485,6 +485,7 @@ struct sdvo_device_mapping { > > struct intel_connector; > struct intel_encoder; > +struct intel_atomic_state; > struct intel_crtc_state; > struct intel_initial_plane_config; > struct intel_crtc; > @@ -498,8 +499,9 @@ struct drm_i915_display_funcs { > int (*compute_intermediate_wm)(struct drm_device *dev, > struct intel_crtc *intel_crtc, > struct intel_crtc_state *newstate); > - void (*initial_watermarks)(struct intel_crtc_state *cstate); > - void (*optimize_watermarks)(struct intel_crtc_state *cstate); > + void (*initial_watermarks)(struct intel_atomic_state *state, struct > intel_crtc_state *cstate); > + void (*atomic_evade_watermarks)(struct intel_atomic_state *state, > struct intel_crtc_state *cstate); > + void (*optimize_watermarks)(struct intel_atomic_state *state, struct > intel_crtc_state *cstate); > int (*compute_global_watermarks)(struct drm_atomic_state *state); > void (*update_wm)(struct drm_crtc *crtc); > int (*modeset_calc_cdclk)(struct drm_atomic_state *state); > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 9ea9a3bc0bc0..48eb0635ec0f 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5170,7 +5170,7 @@ static void intel_pre_plane_update(struct > intel_crtc_state *old_crtc_state) >* us to. >*/ > if (dev_priv->display.initial_watermarks != NULL) > - dev_priv->display.initial_watermarks(pipe_config); > + > dev_priv->display.initial_watermarks(to_intel_atomic_state(old_state), > pipe_config); > else if (pipe_config->update_wm_pre) > intel_update_watermarks(&crtc->base); > } > @@ -5384,7 +5384,7 @@ static void ironlake_crtc_enable(struct > intel_crtc_state *pipe_config, > intel_color_load_luts(&pipe_config->base); > > if (dev_priv->display.initial_watermarks != NULL) > - dev_priv->display.initial_watermarks(intel_crtc->config); > + > dev_priv->display.initial_watermarks(to_intel_atomic_state(old_state), > intel_crtc->config); > intel_enable_pipe(intel_crtc); > > if (intel_crtc->config->has_pch_encoder) > @@ -5490,7 +5490,7 @@ static void haswell_crtc_enable(struct intel_crtc_state > *pipe_config, > intel_ddi_enable_transcoder_func(crtc); > > if (dev_priv->display.initial_watermarks != NULL) > - dev_priv->display.initial_watermarks(pipe_config); > + > dev_priv->display.initial_watermarks(to_intel_atomic_state(old_state), > pipe_config); > else > intel_update_watermarks(crtc); > > @@ -14526,7 +14526,7 @@ static void intel_atomic_commit_tail(struct > drm_atomic_state *state) > intel_cstate = to_intel_crtc_state(crtc->state); > > if (dev_priv->display.optimize_watermarks) > - dev_priv->display.optimize_watermarks(intel_cstate); > + dev_priv->display.optimize_watermarks(intel_state, > intel_cstate); > } > > for_each_crtc_in_state(state, crtc, old_crtc_state, i) { > @@ -14934,7 +14934,6 @@ static void intel_begin_crtc_commit(struct drm_crtc > *crtc, > struct intel_crtc_state *old_in
Re: [Intel-gfx] [PATCH v2 07/11] drm/i915: Add a atomic evasion step to watermark programming, v2.
On Wed, Oct 26, 2016 at 03:41:35PM +0200, Maarten Lankhorst wrote: > Allow the driver to write watermarks during atomic evasion. > This will make it possible to write the watermarks in a cleaner > way on gen9+. > > intel_atomic_state is not used here yet, but will be used when > we program all watermarks as a separate step during evasion. > > This also writes linetime all the time, while before it was only > done during plane updates. This looks like this could be a bugfix, > but I'm not sure what it affects. > > Changes since v1: > - Add comment about atomic evasion to commit message. > - Unwrap I915_WRITE call. (Lyude) > > Signed-off-by: Maarten Lankhorst > Cc: Matt Roper > Cc: Paulo Zanoni > --- > drivers/gpu/drm/i915/i915_drv.h | 6 -- > drivers/gpu/drm/i915/intel_display.c | 20 +--- > drivers/gpu/drm/i915/intel_pm.c | 18 -- > 3 files changed, 29 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 7a621c74254e..7a477d6a486e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -485,6 +485,7 @@ struct sdvo_device_mapping { > > struct intel_connector; > struct intel_encoder; > +struct intel_atomic_state; > struct intel_crtc_state; > struct intel_initial_plane_config; > struct intel_crtc; > @@ -498,8 +499,9 @@ struct drm_i915_display_funcs { > int (*compute_intermediate_wm)(struct drm_device *dev, > struct intel_crtc *intel_crtc, > struct intel_crtc_state *newstate); > - void (*initial_watermarks)(struct intel_crtc_state *cstate); > - void (*optimize_watermarks)(struct intel_crtc_state *cstate); > + void (*initial_watermarks)(struct intel_atomic_state *state, struct > intel_crtc_state *cstate); > + void (*atomic_evade_watermarks)(struct intel_atomic_state *state, > struct intel_crtc_state *cstate); Same drive by comment I gave before somewhere: That name is still super confusing. We're not trying to evade watermarks are we? -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 07/11] drm/i915: Add a atomic evasion step to watermark programming, v2.
Allow the driver to write watermarks during atomic evasion. This will make it possible to write the watermarks in a cleaner way on gen9+. intel_atomic_state is not used here yet, but will be used when we program all watermarks as a separate step during evasion. This also writes linetime all the time, while before it was only done during plane updates. This looks like this could be a bugfix, but I'm not sure what it affects. Changes since v1: - Add comment about atomic evasion to commit message. - Unwrap I915_WRITE call. (Lyude) Signed-off-by: Maarten Lankhorst Cc: Matt Roper Cc: Paulo Zanoni --- drivers/gpu/drm/i915/i915_drv.h | 6 -- drivers/gpu/drm/i915/intel_display.c | 20 +--- drivers/gpu/drm/i915/intel_pm.c | 18 -- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 7a621c74254e..7a477d6a486e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -485,6 +485,7 @@ struct sdvo_device_mapping { struct intel_connector; struct intel_encoder; +struct intel_atomic_state; struct intel_crtc_state; struct intel_initial_plane_config; struct intel_crtc; @@ -498,8 +499,9 @@ struct drm_i915_display_funcs { int (*compute_intermediate_wm)(struct drm_device *dev, struct intel_crtc *intel_crtc, struct intel_crtc_state *newstate); - void (*initial_watermarks)(struct intel_crtc_state *cstate); - void (*optimize_watermarks)(struct intel_crtc_state *cstate); + void (*initial_watermarks)(struct intel_atomic_state *state, struct intel_crtc_state *cstate); + void (*atomic_evade_watermarks)(struct intel_atomic_state *state, struct intel_crtc_state *cstate); + void (*optimize_watermarks)(struct intel_atomic_state *state, struct intel_crtc_state *cstate); int (*compute_global_watermarks)(struct drm_atomic_state *state); void (*update_wm)(struct drm_crtc *crtc); int (*modeset_calc_cdclk)(struct drm_atomic_state *state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9ea9a3bc0bc0..48eb0635ec0f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5170,7 +5170,7 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state) * us to. */ if (dev_priv->display.initial_watermarks != NULL) - dev_priv->display.initial_watermarks(pipe_config); + dev_priv->display.initial_watermarks(to_intel_atomic_state(old_state), pipe_config); else if (pipe_config->update_wm_pre) intel_update_watermarks(&crtc->base); } @@ -5384,7 +5384,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config, intel_color_load_luts(&pipe_config->base); if (dev_priv->display.initial_watermarks != NULL) - dev_priv->display.initial_watermarks(intel_crtc->config); + dev_priv->display.initial_watermarks(to_intel_atomic_state(old_state), intel_crtc->config); intel_enable_pipe(intel_crtc); if (intel_crtc->config->has_pch_encoder) @@ -5490,7 +5490,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, intel_ddi_enable_transcoder_func(crtc); if (dev_priv->display.initial_watermarks != NULL) - dev_priv->display.initial_watermarks(pipe_config); + dev_priv->display.initial_watermarks(to_intel_atomic_state(old_state), pipe_config); else intel_update_watermarks(crtc); @@ -14526,7 +14526,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) intel_cstate = to_intel_crtc_state(crtc->state); if (dev_priv->display.optimize_watermarks) - dev_priv->display.optimize_watermarks(intel_cstate); + dev_priv->display.optimize_watermarks(intel_state, intel_cstate); } for_each_crtc_in_state(state, crtc, old_crtc_state, i) { @@ -14934,7 +14934,6 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc, struct intel_crtc_state *old_intel_state = to_intel_crtc_state(old_crtc_state); bool modeset = needs_modeset(crtc->state); - enum pipe pipe = intel_crtc->pipe; /* Perform vblank evasion around commit operation */ intel_pipe_update_start(intel_crtc); @@ -14947,14 +14946,13 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc, intel_color_load_luts(crtc->state); } - if (intel_cstate->update_pipe) { + if (intel_cstate->update_pipe) intel_update_pipe_config(intel_crtc, old_intel_state); - } else if (INTEL_GEN(dev_priv) >= 9) { + else if (INTEL_GEN(dev_priv) >= 9)