[Intel-gfx] [PATCH 1/8] drm/i915/skl+: Prepare for removing data rate from skl watermark state

[Maarten Lankhorst]

Op 20-10-16 om 00:13 schreef Matt Roper:
> On Wed, Oct 12, 2016 at 03:28:14PM +0200, Maarten Lankhorst wrote:
>> Caching is not required, drm_atomic_crtc_state_for_each_plane_state
>> can be used to inspect all plane_states that are assigned to the
>> current crtc_state, so we can just recalculate every time.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_pm.c | 27 ++++++++++++---------------
>>  1 file changed, 12 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 6af1587e9d84..b96a899c899d 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -31,6 +31,7 @@
>>  #include "intel_drv.h"
>>  #include "../../../platform/x86/intel_ips.h"
>>  #include <linux/module.h>
>> +#include <drm/drm_atomic_helper.h>
>>  
>>  /**
>>   * DOC: RC6
>> @@ -3242,18 +3243,17 @@ skl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate)
>>  	struct drm_crtc *crtc = cstate->crtc;
>>  	struct drm_device *dev = crtc->dev;
>>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> -	const struct drm_plane *plane;
>> +	struct drm_plane *plane;
>>  	const struct intel_plane *intel_plane;
>> -	struct drm_plane_state *pstate;
>> +	const struct drm_plane_state *pstate;
>>  	unsigned int rate, total_data_rate = 0;
>>  	int id;
>> -	int i;
>>  
>>  	if (WARN_ON(!state))
>>  		return 0;
>>  
>>  	/* Calculate and cache data rate for each plane */
>> -	for_each_plane_in_state(state, plane, pstate, i) {
>> +	drm_atomic_crtc_state_for_each_plane_state(plane, pstate, cstate) {
>>  		id = skl_wm_plane_id(to_intel_plane(plane));
>>  		intel_plane = to_intel_plane(plane);
>>  
>> @@ -3356,7 +3356,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>  	struct intel_plane *intel_plane;
>>  	struct drm_plane *plane;
>> -	struct drm_plane_state *pstate;
>> +	const struct drm_plane_state *pstate;
>>  	enum pipe pipe = intel_crtc->pipe;
>>  	struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb;
>>  	uint16_t alloc_size, start, cursor_blocks;
>> @@ -3392,14 +3392,14 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>>  	alloc_size -= cursor_blocks;
>>  
>>  	/* 1. Allocate the mininum required blocks for each active plane */
>> -	for_each_plane_in_state(state, plane, pstate, i) {
>> +	drm_atomic_crtc_state_for_each_plane_state(plane, pstate, &cstate->base) {
>>  		intel_plane = to_intel_plane(plane);
>>  		id = skl_wm_plane_id(intel_plane);
>>  
>>  		if (intel_plane->pipe != pipe)
>>  			continue;
>>  
>> -		if (!to_intel_plane_state(pstate)->base.visible) {
>> +		if (!pstate->visible) {
>>  			minimum[id] = 0;
>>  			y_minimum[id] = 0;
>>  			continue;
>> @@ -3948,7 +3948,7 @@ skl_ddb_add_affected_planes(struct intel_crtc_state *cstate)
>>  
>>  	WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc));
>>  
>> -	drm_for_each_plane_mask(plane, dev, crtc->state->plane_mask) {
>> +	drm_for_each_plane_mask(plane, dev, cstate->base.plane_mask) {
> Is this change necessary?  Any plane that differs between the two masks
> should already be part of our state, so I don't think this changes the
> behavior at all.  The original 'crtc->state->plane_mask' form is closer
> to the drm_atomic_add_affected_planes() that this function is modeled
> after so my slight preference would be to leave it alone for
> consistency.
>
> Aside from that, this patch is
Not completely, but I was removing it since I'm trying to get rid of all pointers to obj->state as much as I can.
All accesses should be through some wrapper functions, still working out the specifics.

~Maarten