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

Maarten Lankhorst
2015-06-18T05:42:12Z

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