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

From: Jani Nikula <jani.nikula@linux.intel.com>
To: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@gmail.com>,
	Chris Wilson <chris@chris-wilson.co.uk>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	Hans de Goede <hdegoede@redhat.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	Daniel Vetter <daniel.vetter@intel.com>, Lyude <cpaul@redhat.com>
Subject: Re: [PATCH] drm/i915: Avoid using dev_priv->info.gen directly.
Date: Mon, 02 Oct 2017 11:33:13 +0300	[thread overview]
Message-ID: <87tvzi54qe.fsf@intel.com> (raw)
In-Reply-To: <1506675938.4729.30.camel@linux.intel.com>

On Fri, 29 Sep 2017, Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote:
> On Wed, 2017-09-27 at 12:32 -0700, Rodrigo Vivi wrote:
>> On Wed, Sep 27, 2017 at 12:03 AM, Jani Nikula
>> <jani.nikula@linux.intel.com> wrote:
>> > 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.
>> 
>> Makes sense.
>> Merged to dinq without the "Fixes:" tags.
>> Thanks for all comments and reviews.
>
> We discussed this with Chris too, I ended up suggesting there could be
> something along; "Backport: none" or "Backport: v4.0+", instead of the
> horrible #v4.0 that is causing pain every now and then, by getting
> mixed to the To: fields in a wrong way.
>
> Any thoughts on that?

The "# v4.0" notation is described in stable-kernel-rules.rst, and if
there are problems with it, they will get fixed. Git screwed it up, and
AFAIK it has since been fixed. I think stick with what the stable team
wants.

> Fixes: is an interesting metric in other sense too, or then just decide
> to use X-Fixes: when we don't want it to end up backported.

IMO Fixes: should only be used for actual functional fixes, not cosmetic
stuff. We don't need to track cosmetic fixes, nobody wants to backport
them anywhere. Our tooling heavily relies on Fixes: to select fixes
backports, and we'll just look silly sending "comment typo fix" style
patches Linus' way at, say, -rc6 when stuff is supposed to abide by
stable-kernel-rules.rst.

BR,
Jani.

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