Re: [PATCH v3 06/19] drm/i915: Split skl_update_scaler, v3.

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Matt Roper <matthew.d.roper@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v3 06/19] drm/i915: Split skl_update_scaler, v3.
Date: Thu, 18 Jun 2015 07:42:10 +0200	[thread overview]
Message-ID: <55825A32.7050903@linux.intel.com> (raw)
In-Reply-To: <20150618014821.GD3607@intel.com>

Op 18-06-15 om 03:48 schreef Matt Roper:
> On Mon, Jun 15, 2015 at 12:33:43PM +0200, Maarten Lankhorst wrote:
>> It's easier to read separate functions for crtc and plane scaler state.
>>
>> Changes since v1:
>>  - Update documentation.
>> Changes since v2:
>>  - Get rid of parameters to skl_update_scaler only used for traces.
>>    This avoids needing to document the other parameters.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 211 +++++++++++++++++++----------------
>>  drivers/gpu/drm/i915/intel_dp.c      |   2 +-
>>  drivers/gpu/drm/i915/intel_drv.h     |  12 +-
>>  drivers/gpu/drm/i915/intel_sprite.c  |   3 +-
>>  4 files changed, 121 insertions(+), 107 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 0f7652a31c95..26d610acb61f 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4303,62 +4303,16 @@ static void cpt_verify_modeset(struct drm_device *dev, int pipe)
>>  	}
>>  }
>>  
>> -/**
>> - * skl_update_scaler_users - Stages update to crtc's scaler state
>> - * @intel_crtc: crtc
>> - * @crtc_state: crtc_state
>> - * @plane: plane (NULL indicates crtc is requesting update)
>> - * @plane_state: plane's state
>> - * @force_detach: request unconditional detachment of scaler
>> - *
>> - * This function updates scaler state for requested plane or crtc.
>> - * To request scaler usage update for a plane, caller shall pass plane pointer.
>> - * To request scaler usage update for crtc, caller shall pass plane pointer
>> - * as NULL.
>> - *
>> - * Return
>> - *     0 - scaler_usage updated successfully
>> - *    error - requested scaling cannot be supported or other error condition
>> - */
>> -int
>> -skl_update_scaler_users(
>> -	struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state,
>> -	struct intel_plane *intel_plane, struct intel_plane_state *plane_state,
>> -	int force_detach)
>> +static int
>> +skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
>> +		  unsigned scaler_idx, int *scaler_id, unsigned int rotation,
>                            ^^^^^^^^^^
> This parameter isn't actually the scaler index is it (that's what
> scaler_id winds up being once assigned here)?  I think this one is the
> plane index that we're assigning a scaler for (or the special value of
> SKL_CRTC_INDEX if we're assigning for the CRTC instead of a plane).
>
> Maybe 'scaler_target' or 'scaler_user' would be better?
>
Could we call it 'i'?

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx