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 <maarten.lankhorst@xxxxxxxxxxxxxxx>
>> ---
>> 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
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
