On Fri, Dec 15, 2017 at 06:07:27AM +0000, Michal Wajdeczko wrote:
> On Thu, 14 Dec 2017 23:30:56 +0100, Srivatsa, Anusha
> <anusha.srivatsa@xxxxxxxxx> wrote:
>
> >
> >
> > > -----Original Message-----
> > > From: Wajdeczko, Michal
> > > Sent: Thursday, December 14, 2017 2:18 PM
> > > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Srivatsa, Anusha
> > > <anusha.srivatsa@xxxxxxxxx>
> > > Cc: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>
> > > Subject: Re: [PATCH] drm/i915/glk: Disable Guc and HuC
> > > on GLK
> > >
> > > On Thu, 14 Dec 2017 22:58:37 +0100, Anusha Srivatsa
> > > <anusha.srivatsa@xxxxxxxxx> wrote:
> > >
> > > > Since the firmwares are released yet to public repo, disable them on
> > > > Geminilake.
> > > >
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
> > > > ---
> > > > drivers/gpu/drm/i915/i915_pci.c | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > > > b/drivers/gpu/drm/i915/i915_pci.c index fa67d3d..ddf7530 100644
> > > > --- a/drivers/gpu/drm/i915/i915_pci.c
> > > > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > > > @@ -521,6 +521,11 @@ static const struct intel_device_info
> > > > intel_geminilake_info __initconst = {
> > > > GEN9_LP_FEATURES,
> > > > .platform = INTEL_GEMINILAKE,
> > > > .ddb_size = 1024,
> > > > + /* FIXME Geminilake supports GuC but currently firmwares
> > > > + * have not made it to public repo. Lets disable the support
> > > > + * as a temporary fix.
> > > > + */
> > > > + .has_guc = 0,
> > >
> > > Maybe better place to put this fix is __get_platform_enable_guc()
> > > like in [1] [1]
> > > https://patchwork.freedesktop.org/patch/192006/
> >
> > Michal,
> > Hmm the reference patch is controlling guc/huc through a module
> > parameter but wont the above approach be neater platform wise?
>
> With your patch it would be impossible to check even preliminary
> firmwares from non-public repo without recompilation of the driver.
>
> While with variant below it would just require changing modparams like:
> enable_guc=1
> guc_firmware_path=preliminary/glk.bin
This is a fair point imo. With has_guc=0 you remove the possibility
of testing preliminary GuC/HuC without a kernel patch...
>
> Note that auto mode (enable_guc=-1) will still keep GuC disabled for GLK.
... but I believe that the main point here is that if
we don't have a public image it doesn't exist from the upstream perspective.
The ideal is not need this patch and releasing firmware sooner. I see same
happening for all upcoming platforms...
So, whatever we decide for this case needs to cover future cases as well.
Thanks,
Rodrigo.
>
> Michal
>
> >
> > Anusha
> > > diff --git a/drivers/gpu/drm/i915/intel_uc.c
> > > b/drivers/gpu/drm/i915/intel_uc.c
> > > index 49bccc9..22b0afe 100644
> > > --- a/drivers/gpu/drm/i915/intel_uc.c
> > > +++ b/drivers/gpu/drm/i915/intel_uc.c
> > > @@ -60,6 +60,8 @@ static int __get_platform_enable_guc(struct
> > > drm_i915_private *dev_priv)
> > > enable_guc |= ENABLE_GUC_LOAD_HUC;
> > >
> > > /* Any platform specific fine-tuning can be done here */
> > > + if (IS_GEMINILAKE(dev_priv))
> > > + enable_guc = 0; /* no firmware on CI machines */
> > >
> > > return enable_guc;
> > > }
> > >
> > >
> > > > GLK_COLORS,
> > > > };
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
