[Intel-gfx] [CI 01/12] drm/i915: Introduce intel_atomic_get_plane_state_after_check(), v2.

[Maarten Lankhorst]

Op 29-10-2019 om 19:35 schreef Ville Syrjälä:
> On Tue, Oct 29, 2019 at 08:22:18AM +0100, Maarten Lankhorst wrote:
>> Use this in all the places where we try to acquire planes after the planes
>> atomic_check().
>>
>> In case of intel_modeset_all_pipes() this is not yet done after atomic_check,
>> but seems like it will be in the future. To add some paranoia, add all planes
>> rather than active planes, because of bigjoiner and planar YUV support having
>> extra planes outside of the core's view that wouldn't be added otherwise.
>>
>> Changes since v1:
>> - Always add all planes, to handle force plane updates to work correctly
>>   with a disabled cursor plane.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_atomic.c   | 41 +++++++++----------
>>  .../gpu/drm/i915/display/intel_atomic_plane.c | 15 +++++++
>>  drivers/gpu/drm/i915/display/intel_cdclk.c    | 15 ++++---
>>  drivers/gpu/drm/i915/display/intel_color.c    |  7 ++--
>>  .../drm/i915/display/intel_display_types.h    |  6 +++
>>  drivers/gpu/drm/i915/intel_pm.c               | 14 ++++---
>>  6 files changed, 62 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
>> index 9cd6d2348a1e..80df6c233581 100644
>> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
>> @@ -313,13 +313,10 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
>>  			       struct intel_crtc *intel_crtc,
>>  			       struct intel_crtc_state *crtc_state)
>>  {
>> -	struct drm_plane *plane = NULL;
>> -	struct intel_plane *intel_plane;
>> -	struct intel_plane_state *plane_state = NULL;
>>  	struct intel_crtc_scaler_state *scaler_state =
>>  		&crtc_state->scaler_state;
>>  	struct drm_atomic_state *drm_state = crtc_state->base.state;
>> -	struct intel_atomic_state *intel_state = to_intel_atomic_state(drm_state);
>> +	struct intel_atomic_state *state = to_intel_atomic_state(drm_state);
>>  	int num_scalers_need;
>>  	int i;
>>  
>> @@ -346,6 +343,7 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
>>  
>>  	/* walkthrough scaler_users bits and start assigning scalers */
>>  	for (i = 0; i < sizeof(scaler_state->scaler_users) * 8; i++) {
>> +		struct intel_plane_state *plane_state = NULL;
>>  		int *scaler_id;
>>  		const char *name;
>>  		int idx;
>> @@ -361,19 +359,16 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
>>  			/* panel fitter case: assign as a crtc scaler */
>>  			scaler_id = &scaler_state->scaler_id;
>>  		} else {
>> -			name = "PLANE";
>> +			struct intel_plane *plane;
>>  
>>  			/* plane scaler case: assign as a plane scaler */
>>  			/* find the plane that set the bit as scaler_user */
>> -			plane = drm_state->planes[i].ptr;
>>  
>>  			/*
>>  			 * to enable/disable hq mode, add planes that are using scaler
>>  			 * into this transaction
>>  			 */
>> -			if (!plane) {
>> -				struct drm_plane_state *state;
>> -
>> +			if (!drm_state->planes[i].ptr) {
>>  				/*
>>  				 * GLK+ scalers don't have a HQ mode so it
>>  				 * isn't necessary to change between HQ and dyn mode
>> @@ -382,24 +377,28 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
>>  				if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
>>  					continue;
>>  
>> -				plane = drm_plane_from_index(&dev_priv->drm, i);
>> -				state = drm_atomic_get_plane_state(drm_state, plane);
>> -				if (IS_ERR(state)) {
>> -					DRM_DEBUG_KMS("Failed to add [PLANE:%d] to drm_state\n",
>> -						plane->base.id);
>> -					return PTR_ERR(state);
>> +				plane = to_intel_plane(drm_plane_from_index(&dev_priv->drm, i));
>> +				plane_state =
>> +					intel_atomic_get_plane_state_after_check(state,
>> +										 crtc_state,
>> +										 plane);
>> +				if (IS_ERR(plane_state)) {
>> +					DRM_DEBUG_KMS("Failed to add [PLANE:%d] to drm_state: %li\n",
>> +						plane->base.base.id, PTR_ERR(plane_state));
>> +					return PTR_ERR(plane_state);
>>  				}
>> +			} else {
>> +				plane = to_intel_plane(drm_state->planes[i].ptr);
>> +				plane_state = intel_atomic_get_new_plane_state(state,
>> +									       plane);
>>  			}
>>  
>> -			intel_plane = to_intel_plane(plane);
>> -			idx = plane->base.id;
>> -
>>  			/* plane on different crtc cannot be a scaler user of this crtc */
>> -			if (WARN_ON(intel_plane->pipe != intel_crtc->pipe))
>> +			if (WARN_ON(plane->pipe != intel_crtc->pipe))
>>  				continue;
>>  
>> -			plane_state = intel_atomic_get_new_plane_state(intel_state,
>> -								       intel_plane);
>> +			name = "PLANE";
>> +			idx = plane->base.base.id;
>>  			scaler_id = &plane_state->scaler_id;
>>  		}
>>  
>> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>> index 98f557a9f8ee..a9e2684c6b6d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>> @@ -402,6 +402,21 @@ void i9xx_update_planes_on_crtc(struct intel_atomic_state *state,
>>  	}
>>  }
>>  
>> +struct intel_plane_state *
>> +intel_atomic_get_plane_state_after_check(struct intel_atomic_state *state,
>> +					 struct intel_crtc_state *new_crtc_state,
>> +					 struct intel_plane *plane)
>> +{
>> +	struct intel_plane_state *plane_state =
>> +		intel_atomic_get_plane_state(state, plane);
>> +
>> +	if (IS_ERR(plane_state))
>> +		return plane_state;
>> +
>> +	new_crtc_state->update_planes |= BIT(plane->id);
>> +	return plane_state;
>> +}
> With the hw state being persistent now I guess we don't need this
> anymore?

Yes, it would stil be useful for annotation though. :)