Re: [PATCH] drm/i915: Avoid using dev_priv->info.gen directly.

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 26 Sep 2017, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Tue, Sep 26, 2017 at 09:21:43PM +0000, Paulo Zanoni wrote:
>> Em Ter, 2017-09-26 às 14:13 -0700, Rodrigo Vivi escreveu:
>> > Let's stop this usage before it spreads so much.
>> > 
>> > 1. This check is not part of usual searches happening when adding
>> > new platform.
>> > 2. There is already a duplication here with INTEL_INFO(dev_priv)->gen
>> > and INTEL_GEN(dev_priv).
>> > 
>> > So let's please avoid yet another way.
>> > 
>> > Fixes: b22ca995ba1c ("drm/i915: prepare pipe for YCBCR420 output")
>> > Fixes: 27082493e9c6 ("drm/i915/skl: Update DDB values atomically with
>> > wms/plane attrs")
>> 
>> Not sure if the Fixes tags are appropriate since this is not a bug fix.
>
> I wondered that... but since "dim fixes" provided me that tag along with the
> list of people I should cc I decided to include here. I thought it
> wouldn't hurt and also maybe good to propagate that to everywhere possible so
> we don't recieve more code based on that usage.
>
> But I won't merge today to give time to get Jani's view on that.

Please only use Fixes: for functional fixes that need to be
backported. Like, nobody's going to be happier running a kernel they
know uses INTEL_GEN() consistently.

BR,
Jani.


>
>> 
>> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> 
>> > Cc: Lyude <cpaul@redhat.com>
>> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > Cc: Daniel Vetter <daniel.vetter@intel.com>
>> > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
>> > Cc: Hans de Goede <hdegoede@redhat.com>
>> > Cc: Matt Roper <matthew.d.roper@intel.com>
>> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> > Cc: Imre Deak <imre.deak@intel.com>
>> > Cc: Shashank Sharma <shashank.sharma@intel.com>
>> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_display.c | 6 +++---
>> >  1 file changed, 3 insertions(+), 3 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c
>> > b/drivers/gpu/drm/i915/intel_display.c
>> > index 487b43ba3139..fba76f1bb6dd 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -9310,11 +9310,11 @@ static bool haswell_get_pipe_config(struct
>> > intel_crtc *crtc,
>> >  	pipe_config->gamma_mode =
>> >  		I915_READ(GAMMA_MODE(crtc->pipe)) &
>> > GAMMA_MODE_MODE_MASK;
>> >  
>> > -	if (IS_BROADWELL(dev_priv) || dev_priv->info.gen >= 9) {
>> > +	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) {
>> >  		u32 tmp = I915_READ(PIPEMISC(crtc->pipe));
>> >  		bool clrspace_yuv = tmp &
>> > PIPEMISC_OUTPUT_COLORSPACE_YUV;
>> >  
>> > -		if (IS_GEMINILAKE(dev_priv) || dev_priv->info.gen >=
>> > 10) {
>> > +		if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv)
>> > >= 10) {
>> >  			bool blend_mode_420 = tmp &
>> >  					      PIPEMISC_YUV420_MODE_F
>> > ULL_BLEND;
>> >  
>> > @@ -14217,7 +14217,7 @@ void intel_init_display_hooks(struct
>> > drm_i915_private *dev_priv)
>> >  		dev_priv->display.fdi_link_train =
>> > hsw_fdi_link_train;
>> >  	}
>> >  
>> > -	if (dev_priv->info.gen >= 9)
>> > +	if (INTEL_GEN(dev_priv) >= 9)
>> >  		dev_priv->display.update_crtcs = skl_update_crtcs;
>> >  	else
>> >  		dev_priv->display.update_crtcs = intel_update_crtcs;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx