Re: [PATCH] drm/i915/guc: Removed unused GuC parameters.

From: John Spotswood <john.a.spotswood@intel.com>
To: "Piorkowski, Piotr" <piotr.piorkowski@intel.com>,
	"Thierry, Michel" <michel.thierry@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"chris@chris-wilson.co.uk" <chris@chris-wilson.co.uk>,
	"Kamble, Sagar A" <sagar.a.kamble@intel.com>
Subject: Re: [PATCH] drm/i915/guc: Removed unused GuC parameters.
Date: Tue, 06 Mar 2018 15:50:04 -0800	[thread overview]
Message-ID: <1520380204.9578.10.camel@intel.com> (raw)
In-Reply-To: <1520248363.2906.10.camel@intel.com>

On Mon, 2018-03-05 at 03:12 -0800, Piorkowski, Piotr wrote:
> On Fri, 2018-03-02 at 12:53 +0530, Sagar Arun Kamble wrote:
> > 
> > 
> > On 3/2/2018 12:44 AM, John Spotswood wrote:
> > > 
> > > On Thu, 2018-03-01 at 17:35 +0530, Sagar Arun Kamble wrote:
> > > > 
> > > > On 3/1/2018 1:32 PM, Chris Wilson wrote:
> > > > > 
> > > > > Quoting Michel Thierry (2018-02-28 22:07:51)
> > > > > > 
> > > > > > On 28/02/18 12:26, Michel Thierry wrote:
> > > > > > > 
> > > > > > > On 28/02/18 10:42, Piotr Piórkowski wrote:
> > > > > > > > 
> > > > > > > > In the i915 driver, there is a function,
> > > > > > > > intel_guc_init_params(),
> > > > > > > > which initializes the GuC parameter block which is
> > > > > > > > passed
> > > > > > > > into
> > > > > > > > the GuC. There is parameter GUC_CTL_DEVICE_INFO with
> > > > > > > > values
> > > > > > > > GfxGtType and GfxCoreFamily unused by GuC.
> > > > > > > > 
> > > > > > > > This patch remove GUC_CTL_DEVICE_INFO with GfxGtType
> > > > > > > > and
> > > > > > > > GfxCoreFamily parameters and also unnecessary functions
> > > > > > > > get_gt_type() and get_core_family().
> > > > > > > > 
> > > > > > > Hi,
> > > > > > > 
> > > > > > > Looking at the fw code, you're partially right, GfxGtType
> > > > > > > is
> > > > > > > ignored...
> > > > > > > but GfxCoreFamily isn't.
> > > > > > > 
> > > > > > Unless whoever wrote the fw was smart enough to forget to
> > > > > > call
> > > > > > the
> > > > > > function that is reading GfxCoreFamily... I didn't count on
> > > > > > that.
> > > > > Is the intention to use GfxCoreFamily documented, i.e. are
> > > > > they
> > > > > expecting it part of the interface and may re-instantiate the
> > > > > check
> > > > > "because it was always supposed to exist" in some future
> > > > > version?
> > > > Usage of GfxCoreFamily is only in SLPC and for platform
> > > > specific
> > > > initialization and might be removed in future interfaces.
> > > > If needed, we can add as part of SLPC patches.
> > > Michel and I have traced through the FW code, and both parameters
> > > are
> > > unused.  GfxCoreFamily does appear to be set in the FW, and it
> > > gets
> > > passed into SLPC, but then it never gets used.
> > Hi John,
> > 
> > It is needed for SLPC initialization. Verified on v9 GuC firmware
> > that 
> > SLPC GTPERF gets disabled if i915 does not send this param.
> > We can add this param as part of SLPC patches for GuC versions
> > which 
> > need them.
> Ok, so I think that we should remove this param from i915, and than
> if 
> it is needed, we can add this param as part of SLPC patches, as Sagar
> said.
I have gone back and taken another look at the FW code, and Sagar is
correct. There is a link there.  Apologies for the mistake.  With that
in mind, I'm not clear why we would remove the parameter only to add it
back with the SLPC patches.
> 
> -Piotr 
> > 
> > 
> > Thanks
> > Sagar
> > > 
> > >    I have confirmed with
> > > FW developers that these parameters have been removed for future
> > > gens.
> > > > 
> > > > > 
> > > > > -Chris
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx