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

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Jun 18, 2015 at 07:42:10AM +0200, Maarten Lankhorst wrote:
> 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'?

Not for a function argument really ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx