| Message ID | 1480953869-25267-1-git-send-email-arkadiusz.hiler@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Reviewed-by: Jeff McGee <jeff.mcgee@intel.com> On Mon, Dec 05, 2016 at 05:04:29PM +0100, Arkadiusz Hiler wrote: > The firmware interface file was initially partially autogenerated, but > this is no longer the case. > > It was never updated automatically, and a lot manual changes were > introduced since. > > From now on any changes to the firmware interface will be managed by > hand, which gives us flexibility when it comes to structure reuse > (HuC/GuC) and naming conventions. > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > Cc: Jeff Mcgee <jeff.mcgee@intel.com> > Cc: Sagar A. Kamble <sagar.a.kamble@intel.com> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> > --- > drivers/gpu/drm/i915/intel_guc_fwif.h | 9 --------- > 1 file changed, 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h > index 00ca0df..3202b32 100644 > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h > @@ -23,15 +23,6 @@ > #ifndef _INTEL_GUC_FWIF_H > #define _INTEL_GUC_FWIF_H > > -/* > - * This file is partially autogenerated, although currently with some manual > - * fixups afterwards. In future, it should be entirely autogenerated, in order > - * to ensure that the definitions herein remain in sync with those used by the > - * GuC's own firmware. > - * > - * EDITING THIS FILE IS THEREFORE NOT RECOMMENDED - YOUR CHANGES MAY BE LOST. > - */ > - > #define GFXCORE_FAMILY_GEN9 12 > #define GFXCORE_FAMILY_UNKNOWN 0x7fffffff > > -- > 2.7.4 >
>-----Original Message----- >From: Hiler, Arkadiusz >Sent: Monday, December 5, 2016 8:04 AM >To: intel-gfx@lists.freedesktop.org >Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; Mcgee, Jeff ><jeff.mcgee@intel.com>; Kamble, Sagar A <sagar.a.kamble@intel.com> >Subject: [PATCH] drm/i915/guc: Drop comment on fwif autogeneration > >The firmware interface file was initially partially autogenerated, but this is no >longer the case. > >It was never updated automatically, and a lot manual changes were introduced >since. > >From now on any changes to the firmware interface will be managed by hand, >which gives us flexibility when it comes to structure reuse >(HuC/GuC) and naming conventions. > >Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> >Cc: Jeff Mcgee <jeff.mcgee@intel.com> >Cc: Sagar A. Kamble <sagar.a.kamble@intel.com> >Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> >--- > drivers/gpu/drm/i915/intel_guc_fwif.h | 9 --------- > 1 file changed, 9 deletions(-) > >diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h >b/drivers/gpu/drm/i915/intel_guc_fwif.h >index 00ca0df..3202b32 100644 >--- a/drivers/gpu/drm/i915/intel_guc_fwif.h >+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h >@@ -23,15 +23,6 @@ > #ifndef _INTEL_GUC_FWIF_H > #define _INTEL_GUC_FWIF_H > >-/* >- * This file is partially autogenerated, although currently with some manual >- * fixups afterwards. In future, it should be entirely autogenerated, in order >- * to ensure that the definitions herein remain in sync with those used by the >- * GuC's own firmware. >- * >- * EDITING THIS FILE IS THEREFORE NOT RECOMMENDED - YOUR CHANGES >MAY BE LOST. >- */ With this removal of comment, do you feel moving the contents of intel_guc_fwif.h to intel_uc.c or renaming the file to intel_uc_fwif.h makes a lot of difference? Cheers, Anusha > #define GFXCORE_FAMILY_GEN9 12 > #define GFXCORE_FAMILY_UNKNOWN 0x7fffffff > >-- >2.7.4
On Tue, Dec 06, 2016 at 12:59:03AM +0100, Srivatsa, Anusha wrote: > >-----Original Message----- > >From: Hiler, Arkadiusz > >Sent: Monday, December 5, 2016 8:04 AM > >To: intel-gfx@lists.freedesktop.org > >Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; Mcgee, Jeff > ><jeff.mcgee@intel.com>; Kamble, Sagar A <sagar.a.kamble@intel.com> > >Subject: [PATCH] drm/i915/guc: Drop comment on fwif autogeneration > > > >The firmware interface file was initially partially autogenerated, but this is no > >longer the case. > > > >It was never updated automatically, and a lot manual changes were introduced > >since. > > > >From now on any changes to the firmware interface will be managed by hand, > >which gives us flexibility when it comes to structure reuse > >(HuC/GuC) and naming conventions. > > > >Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > >Cc: Jeff Mcgee <jeff.mcgee@intel.com> > >Cc: Sagar A. Kamble <sagar.a.kamble@intel.com> > >Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> > >--- > > drivers/gpu/drm/i915/intel_guc_fwif.h | 9 --------- > > 1 file changed, 9 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h > >b/drivers/gpu/drm/i915/intel_guc_fwif.h > >index 00ca0df..3202b32 100644 > >--- a/drivers/gpu/drm/i915/intel_guc_fwif.h > >+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h > >@@ -23,15 +23,6 @@ > > #ifndef _INTEL_GUC_FWIF_H > > #define _INTEL_GUC_FWIF_H > > > >-/* > >- * This file is partially autogenerated, although currently with some manual > >- * fixups afterwards. In future, it should be entirely autogenerated, in order > >- * to ensure that the definitions herein remain in sync with those used by the > >- * GuC's own firmware. > >- * > >- * EDITING THIS FILE IS THEREFORE NOT RECOMMENDED - YOUR CHANGES > >MAY BE LOST. > >- */ > > With this removal of comment, do you feel moving the contents of intel_guc_fwif.h to intel_uc.c or renaming the file to intel_uc_fwif.h makes a lot of difference? I think this area could use some reorganization, but we should give it more thought before we do anything. Keeping all the GuC-related defines/structs that are used to communicate with FW and are prone to change in a separate file seems like a good idea. This would make them easier to manage in cases of any turbulence. I agree that `intel_uc_fwif.h` would be a good name, as your HuC patchset is reusing some of the intrfaces (e.g. css headers) for HuC. The rename can be done as part of the HuC effort. I'll give it some more thought today and send you a RFC version of the reorg to include in your patchset.
On Mon, Dec 05, 2016 at 06:59:01PM +0000, Patchwork wrote: > == Series Details == > > Series: drm/i915/guc: Drop comment on fwif autogeneration > URL : https://patchwork.freedesktop.org/series/16373/ > State : warning > > == Summary == > > Series 16373v1 drm/i915/guc: Drop comment on fwif autogeneration > https://patchwork.freedesktop.org/api/1.0/series/16373/revisions/1/mbox/ > > Test gem_busy: > Subgroup basic-hang-default: > fail -> PASS (fi-hsw-4770r) The cange is only a comment removal. Could not affect the pass. > Test kms_pipe_crc_basic: > Subgroup suspend-read-crc-pipe-a: > pass -> DMESG-WARN (fi-skl-6770hq) > [ 408.780897] [drm:skl_set_cdclk [i915]] *ERROR* failed to inform PCU about cdclk change https://bugs.freedesktop.org/show_bug.cgi?id=97929 > > a947529bb652fdcf50e8c0e8ee5102b737bae23e drm-tip: 2016y-12m-05d-14h-27m-06s UTC integration manifest > abb5dbd drm/i915/guc: Drop comment on fwif autogeneration > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3196/
>-----Original Message----- >From: Hiler, Arkadiusz >Sent: Monday, December 5, 2016 9:37 PM >To: Srivatsa, Anusha <anusha.srivatsa@intel.com> >Cc: intel-gfx@lists.freedesktop.org; Mcgee, Jeff <jeff.mcgee@intel.com>; >Kamble, Sagar A <sagar.a.kamble@intel.com> >Subject: Re: [PATCH] drm/i915/guc: Drop comment on fwif autogeneration > >On Tue, Dec 06, 2016 at 12:59:03AM +0100, Srivatsa, Anusha wrote: >> >-----Original Message----- >> >From: Hiler, Arkadiusz >> >Sent: Monday, December 5, 2016 8:04 AM >> >To: intel-gfx@lists.freedesktop.org >> >Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; Mcgee, Jeff >> ><jeff.mcgee@intel.com>; Kamble, Sagar A <sagar.a.kamble@intel.com> >> >Subject: [PATCH] drm/i915/guc: Drop comment on fwif autogeneration >> > >> >The firmware interface file was initially partially autogenerated, >> >but this is no longer the case. >> > >> >It was never updated automatically, and a lot manual changes were >> >introduced since. >> > >> >From now on any changes to the firmware interface will be managed by >> >hand, which gives us flexibility when it comes to structure reuse >> >(HuC/GuC) and naming conventions. >> > >> >Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> >> >Cc: Jeff Mcgee <jeff.mcgee@intel.com> >> >Cc: Sagar A. Kamble <sagar.a.kamble@intel.com> >> >Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> >> >--- >> > drivers/gpu/drm/i915/intel_guc_fwif.h | 9 --------- >> > 1 file changed, 9 deletions(-) >> > >> >diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h >> >b/drivers/gpu/drm/i915/intel_guc_fwif.h >> >index 00ca0df..3202b32 100644 >> >--- a/drivers/gpu/drm/i915/intel_guc_fwif.h >> >+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h >> >@@ -23,15 +23,6 @@ >> > #ifndef _INTEL_GUC_FWIF_H >> > #define _INTEL_GUC_FWIF_H >> > >> >-/* >> >- * This file is partially autogenerated, although currently with >> >some manual >> >- * fixups afterwards. In future, it should be entirely >> >autogenerated, in order >> >- * to ensure that the definitions herein remain in sync with those >> >used by the >> >- * GuC's own firmware. >> >- * >> >- * EDITING THIS FILE IS THEREFORE NOT RECOMMENDED - YOUR CHANGES >MAY >> >BE LOST. >> >- */ >> >> With this removal of comment, do you feel moving the contents of >intel_guc_fwif.h to intel_uc.c or renaming the file to intel_uc_fwif.h makes a lot >of difference? > >I think this area could use some reorganization, but we should give it more >thought before we do anything. > >Keeping all the GuC-related defines/structs that are used to communicate with >FW and are prone to change in a separate file seems like a good idea. > >This would make them easier to manage in cases of any turbulence. > >I agree that `intel_uc_fwif.h` would be a good name, as your HuC patchset is >reusing some of the intrfaces (e.g. css headers) for HuC. > >The rename can be done as part of the HuC effort. I'll give it some more thought >today and send you a RFC version of the reorg to include in your patchset. > Sure. Thanks! Anusha >Cheers, >Arek
On 06/12/2016 05:41, Arkadiusz Hiler wrote: > On Mon, Dec 05, 2016 at 06:59:01PM +0000, Patchwork wrote: >> == Series Details == >> >> Series: drm/i915/guc: Drop comment on fwif autogeneration >> URL : https://patchwork.freedesktop.org/series/16373/ >> State : warning >> >> == Summary == >> >> Series 16373v1 drm/i915/guc: Drop comment on fwif autogeneration >> https://patchwork.freedesktop.org/api/1.0/series/16373/revisions/1/mbox/ >> >> Test gem_busy: >> Subgroup basic-hang-default: >> fail -> PASS (fi-hsw-4770r) > > The cange is only a comment removal. Could not affect the pass. > >> Test kms_pipe_crc_basic: >> Subgroup suspend-read-crc-pipe-a: >> pass -> DMESG-WARN (fi-skl-6770hq) >> > > [ 408.780897] [drm:skl_set_cdclk [i915]] *ERROR* failed to inform PCU about cdclk change > > https://bugs.freedesktop.org/show_bug.cgi?id=97929 > >> >> a947529bb652fdcf50e8c0e8ee5102b737bae23e drm-tip: 2016y-12m-05d-14h-27m-06s UTC integration manifest >> abb5dbd drm/i915/guc: Drop comment on fwif autogeneration Pushed to dinq, thanks for the patch and review! Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h index 00ca0df..3202b32 100644 --- a/drivers/gpu/drm/i915/intel_guc_fwif.h +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h @@ -23,15 +23,6 @@ #ifndef _INTEL_GUC_FWIF_H #define _INTEL_GUC_FWIF_H -/* - * This file is partially autogenerated, although currently with some manual - * fixups afterwards. In future, it should be entirely autogenerated, in order - * to ensure that the definitions herein remain in sync with those used by the - * GuC's own firmware. - * - * EDITING THIS FILE IS THEREFORE NOT RECOMMENDED - YOUR CHANGES MAY BE LOST. - */ - #define GFXCORE_FAMILY_GEN9 12 #define GFXCORE_FAMILY_UNKNOWN 0x7fffffff
The firmware interface file was initially partially autogenerated, but this is no longer the case. It was never updated automatically, and a lot manual changes were introduced since. From now on any changes to the firmware interface will be managed by hand, which gives us flexibility when it comes to structure reuse (HuC/GuC) and naming conventions. Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> Cc: Jeff Mcgee <jeff.mcgee@intel.com> Cc: Sagar A. Kamble <sagar.a.kamble@intel.com> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> --- drivers/gpu/drm/i915/intel_guc_fwif.h | 9 --------- 1 file changed, 9 deletions(-)