Re: [PATCH 4/4] drm/i915: Reorganize sprite init

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Oct 27, 2016 at 09:07:13AM +0200, Daniel Vetter wrote:
> On Tue, Oct 25, 2016 at 06:58:03PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Kill the switch statement from the sprite init code and replace with a
> > more straightforward if ladder. Now each significant evolution of the
> > sprite hardware is in its own neat box.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_sprite.c | 70 ++++++++++++++++---------------------
> >  1 file changed, 31 insertions(+), 39 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 41ae7f562eec..70b50a27763e 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1067,25 +1067,25 @@ intel_sprite_plane_create(struct drm_device *dev, enum pipe pipe, int plane)
> >  	}
> >  	intel_plane->base.state = &state->base;
> >  
> > -	switch (INTEL_INFO(dev)->gen) {
> > -	case 5:
> > -	case 6:
> > +	if (INTEL_GEN(dev_priv) >= 9) {
> 
> Maybe do an s/dev/dev_priv/ on the function paramaters while at it? With
> or wihtout that:

I went without. Looks like the change would like to propagate quite a
bit further, so I figured I'd so a separate series for it.

> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Series pushed to dinq. Thanls for the review.

> 
> 
> >  		intel_plane->can_scale = true;
> > -		intel_plane->max_downscale = 16;
> > -		intel_plane->update_plane = ilk_update_plane;
> > -		intel_plane->disable_plane = ilk_disable_plane;
> > +		state->scaler_id = -1;
> >  
> > -		if (IS_GEN6(dev_priv)) {
> > -			plane_formats = snb_plane_formats;
> > -			num_plane_formats = ARRAY_SIZE(snb_plane_formats);
> > -		} else {
> > -			plane_formats = ilk_plane_formats;
> > -			num_plane_formats = ARRAY_SIZE(ilk_plane_formats);
> > -		}
> > -		break;
> > +		intel_plane->update_plane = skl_update_plane;
> > +		intel_plane->disable_plane = skl_disable_plane;
> > +
> > +		plane_formats = skl_plane_formats;
> > +		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
> > +	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> > +		intel_plane->can_scale = false;
> > +		intel_plane->max_downscale = 1;
> > +
> > +		intel_plane->update_plane = vlv_update_plane;
> > +		intel_plane->disable_plane = vlv_disable_plane;
> >  
> > -	case 7:
> > -	case 8:
> > +		plane_formats = vlv_plane_formats;
> > +		num_plane_formats = ARRAY_SIZE(vlv_plane_formats);
> > +	} else if (INTEL_GEN(dev_priv) >= 7) {
> >  		if (IS_IVYBRIDGE(dev_priv)) {
> >  			intel_plane->can_scale = true;
> >  			intel_plane->max_downscale = 2;
> > @@ -1094,33 +1094,25 @@ intel_sprite_plane_create(struct drm_device *dev, enum pipe pipe, int plane)
> >  			intel_plane->max_downscale = 1;
> >  		}
> >  
> > -		if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> > -			intel_plane->update_plane = vlv_update_plane;
> > -			intel_plane->disable_plane = vlv_disable_plane;
> > +		intel_plane->update_plane = ivb_update_plane;
> > +		intel_plane->disable_plane = ivb_disable_plane;
> >  
> > -			plane_formats = vlv_plane_formats;
> > -			num_plane_formats = ARRAY_SIZE(vlv_plane_formats);
> > -		} else {
> > -			intel_plane->update_plane = ivb_update_plane;
> > -			intel_plane->disable_plane = ivb_disable_plane;
> > +		plane_formats = snb_plane_formats;
> > +		num_plane_formats = ARRAY_SIZE(snb_plane_formats);
> > +	} else {
> > +		intel_plane->can_scale = true;
> > +		intel_plane->max_downscale = 16;
> > +
> > +		intel_plane->update_plane = ilk_update_plane;
> > +		intel_plane->disable_plane = ilk_disable_plane;
> >  
> > +		if (IS_GEN6(dev_priv)) {
> >  			plane_formats = snb_plane_formats;
> >  			num_plane_formats = ARRAY_SIZE(snb_plane_formats);
> > +		} else {
> > +			plane_formats = ilk_plane_formats;
> > +			num_plane_formats = ARRAY_SIZE(ilk_plane_formats);
> >  		}
> > -		break;
> > -	case 9:
> > -		intel_plane->can_scale = true;
> > -		intel_plane->update_plane = skl_update_plane;
> > -		intel_plane->disable_plane = skl_disable_plane;
> > -		state->scaler_id = -1;
> > -
> > -		plane_formats = skl_plane_formats;
> > -		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
> > -		break;
> > -	default:
> > -		MISSING_CASE(INTEL_INFO(dev)->gen);
> > -		ret = -ENODEV;
> > -		goto fail;
> >  	}
> >  
> >  	if (INTEL_GEN(dev_priv) >= 9) {
> > @@ -1139,7 +1131,7 @@ intel_sprite_plane_create(struct drm_device *dev, enum pipe pipe, int plane)
> >  
> >  	possible_crtcs = (1 << pipe);
> >  
> > -	if (INTEL_INFO(dev)->gen >= 9)
> > +	if (INTEL_GEN(dev_priv) >= 9)
> >  		ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
> >  					       &intel_plane_funcs,
> >  					       plane_formats, num_plane_formats,
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx