Hey, Op 20-10-16 om 01:15 schreef Matt Roper: > On Wed, Oct 12, 2016 at 03:28:18PM +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+. >> >> Signed-off-by: Maarten Lankhorst <[email protected]> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 6 ++++-- >> drivers/gpu/drm/i915/intel_display.c | 18 ++++++++---------- >> drivers/gpu/drm/i915/intel_pm.c | 19 +++++++++++++++++-- >> 3 files changed, 29 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h >> index f65ccf9b0bea..09588c58148f 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -484,6 +484,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; >> @@ -497,8 +498,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); > initial_watermarks() and optimize_watermarks() are currently only used > on ILK (and possibly by in-development VLV/CHV patches that Ville is > working on?). As far as I can see, the top-level state that we add as a > parameter here doesn't actually get used in the implementations. Are > you adding it to just make them more similar to the signature of the new > atomic_evade_watermarks vfunc or did you have something else in mind? > > I'd also suggest adding a brief comment to your new skl_evade_crtc_wm() > function that indicates that nearly all of the gen9 watermark values are > per-plane values that get written as part of the general plane update in > skylake_update_primary_plane and/or skl_update_plane. Given that those > two functions are located in other files that may help clarify to future > developers why this function appears so trivial.
We don't completely use intel_atomic_state here yet, patch 7 uses it for the ddb allocation and because initial_watermarks ends up calling .atomic_evade_watermarks(). I added intel_atomic_state to all callbacks to keep the function signature identical. It looked better to me to put the function signature in a small change, and then the behavioral change in patch 7 separately, for easier bisection. ~Maarten _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
