| Message ID | 20241011103250.1035316-4-raag.jadav@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Implement Wa_14022698537 | expand |
On Fri, 11 Oct 2024, Raag Jadav <raag.jadav@intel.com> wrote: > Having similar naming convention in intel-family.h and intel_device_info.h > results in redefinition of a few platforms. Define CPU IDs in its own file > to avoid this. > > Signed-off-by: Raag Jadav <raag.jadav@intel.com> > --- > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/gt/intel_wa_cpu.c | 34 +++++++++++++++++++++ > drivers/gpu/drm/i915/gt/intel_workarounds.h | 2 ++ > 3 files changed, 37 insertions(+) > create mode 100644 drivers/gpu/drm/i915/gt/intel_wa_cpu.c > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index c63fa2133ccb..1f9b503ab976 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -121,6 +121,7 @@ gt-y += \ > gt/intel_timeline.o \ > gt/intel_tlb.o \ > gt/intel_wopcm.o \ > + gt/intel_wa_cpu.o \ > gt/intel_workarounds.o \ > gt/shmem_utils.o \ > gt/sysfs_engines.o > diff --git a/drivers/gpu/drm/i915/gt/intel_wa_cpu.c b/drivers/gpu/drm/i915/gt/intel_wa_cpu.c > new file mode 100644 > index 000000000000..cbdab13e9db6 > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/intel_wa_cpu.c > @@ -0,0 +1,34 @@ > +// SPDX-License-Identifier: MIT > +/* > + * Copyright © 2024 Intel Corporation > + * > + * This file is introduced to avoid platform redefinition from > + * intel_device_info.h :( I think the comment is good to have, but please be more specific and direct, and leave out the emoticons. "Avoid INTEL_<PLATFORM> name collisions between asm/intel-family.h and intel_device_info.h by having a separate file." Or something like that. Spells out exactly what the problem is, instead of leaving the reader guessing. > + */ > + > +#include "intel_workarounds.h" > + > +#ifdef CONFIG_X86 > +#include <asm/cpu_device_id.h> > +#include <asm/intel-family.h> > + > +static const struct x86_cpu_id wa_cpu_ids[] = { > + X86_MATCH_VFM(INTEL_ALDERLAKE, NULL), > + X86_MATCH_VFM(INTEL_ALDERLAKE_L, NULL), > + X86_MATCH_VFM(INTEL_COMETLAKE, NULL), > + X86_MATCH_VFM(INTEL_KABYLAKE, NULL), > + X86_MATCH_VFM(INTEL_KABYLAKE_L, NULL), > + X86_MATCH_VFM(INTEL_RAPTORLAKE, NULL), > + X86_MATCH_VFM(INTEL_RAPTORLAKE_P, NULL), > + X86_MATCH_VFM(INTEL_RAPTORLAKE_S, NULL), > + X86_MATCH_VFM(INTEL_ROCKETLAKE, NULL), > + {} > +}; > + > +bool intel_match_wa_cpu(void) IMO the name's too generic. > +{ > + return x86_match_cpu(wa_cpu_ids); > +} > +#else > +bool intel_match_wa_cpu(void) { return false; } > +#endif > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.h b/drivers/gpu/drm/i915/gt/intel_workarounds.h > index 9beaab77c7f0..12f24fb31363 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.h > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.h > @@ -21,6 +21,8 @@ static inline void intel_wa_list_free(struct i915_wa_list *wal) > memset(wal, 0, sizeof(*wal)); > } > > +bool intel_match_wa_cpu(void); > + > void intel_engine_init_ctx_wa(struct intel_engine_cs *engine); > int intel_engine_emit_ctx_wa(struct i915_request *rq);
Hi Raag On 10/11/2024 4:02 PM, Raag Jadav wrote: > Having similar naming convention in intel-family.h and intel_device_info.h > results in redefinition of a few platforms. Define CPU IDs in its own file > to avoid this. > > Signed-off-by: Raag Jadav <raag.jadav@intel.com> > --- > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/gt/intel_wa_cpu.c | 34 +++++++++++++++++++++ > drivers/gpu/drm/i915/gt/intel_workarounds.h | 2 ++ > 3 files changed, 37 insertions(+) > create mode 100644 drivers/gpu/drm/i915/gt/intel_wa_cpu.c > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index c63fa2133ccb..1f9b503ab976 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -121,6 +121,7 @@ gt-y += \ > gt/intel_timeline.o \ > gt/intel_tlb.o \ > gt/intel_wopcm.o \ > + gt/intel_wa_cpu.o \ Why is this file under gt/ ? Doesn't seem to be gt specific > gt/intel_workarounds.o \ > gt/shmem_utils.o \ > gt/sysfs_engines.o > diff --git a/drivers/gpu/drm/i915/gt/intel_wa_cpu.c b/drivers/gpu/drm/i915/gt/intel_wa_cpu.c > new file mode 100644 > index 000000000000..cbdab13e9db6 > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/intel_wa_cpu.c > @@ -0,0 +1,34 @@ > +// SPDX-License-Identifier: MIT > +/* > + * Copyright © 2024 Intel Corporation > + * > + * This file is introduced to avoid platform redefinition from > + * intel_device_info.h :( > + */ > + > +#include "intel_workarounds.h" > + > +#ifdef CONFIG_X86 > +#include <asm/cpu_device_id.h> > +#include <asm/intel-family.h> > + > +static const struct x86_cpu_id wa_cpu_ids[] = { > + X86_MATCH_VFM(INTEL_ALDERLAKE, NULL), > + X86_MATCH_VFM(INTEL_ALDERLAKE_L, NULL), > + X86_MATCH_VFM(INTEL_COMETLAKE, NULL), > + X86_MATCH_VFM(INTEL_KABYLAKE, NULL), > + X86_MATCH_VFM(INTEL_KABYLAKE_L, NULL), > + X86_MATCH_VFM(INTEL_RAPTORLAKE, NULL), > + X86_MATCH_VFM(INTEL_RAPTORLAKE_P, NULL), > + X86_MATCH_VFM(INTEL_RAPTORLAKE_S, NULL), > + X86_MATCH_VFM(INTEL_ROCKETLAKE, NULL), > + {} > +}; > + Add a doc Thanks, Riana > +bool intel_match_wa_cpu(void) > +{ > + return x86_match_cpu(wa_cpu_ids); > +} > +#else > +bool intel_match_wa_cpu(void) { return false; } > +#endif > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.h b/drivers/gpu/drm/i915/gt/intel_workarounds.h > index 9beaab77c7f0..12f24fb31363 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.h > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.h > @@ -21,6 +21,8 @@ static inline void intel_wa_list_free(struct i915_wa_list *wal) > memset(wal, 0, sizeof(*wal)); > } > > +bool intel_match_wa_cpu(void); > + > void intel_engine_init_ctx_wa(struct intel_engine_cs *engine); > int intel_engine_emit_ctx_wa(struct i915_request *rq); >
On Tue, Oct 15, 2024 at 03:56:10PM +0530, Riana Tauro wrote: > Hi Raag > > On 10/11/2024 4:02 PM, Raag Jadav wrote: > > Having similar naming convention in intel-family.h and intel_device_info.h > > results in redefinition of a few platforms. Define CPU IDs in its own file > > to avoid this. > > > > Signed-off-by: Raag Jadav <raag.jadav@intel.com> > > --- > > drivers/gpu/drm/i915/Makefile | 1 + > > drivers/gpu/drm/i915/gt/intel_wa_cpu.c | 34 +++++++++++++++++++++ > > drivers/gpu/drm/i915/gt/intel_workarounds.h | 2 ++ > > 3 files changed, 37 insertions(+) > > create mode 100644 drivers/gpu/drm/i915/gt/intel_wa_cpu.c > > > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > > index c63fa2133ccb..1f9b503ab976 100644 > > --- a/drivers/gpu/drm/i915/Makefile > > +++ b/drivers/gpu/drm/i915/Makefile > > @@ -121,6 +121,7 @@ gt-y += \ > > gt/intel_timeline.o \ > > gt/intel_tlb.o \ > > gt/intel_wopcm.o \ > > + gt/intel_wa_cpu.o \ > Why is this file under gt/ ? Doesn't seem to be gt specific The idea is to couple it with intel_workarounds.c Any other place it'd rather be? > > gt/intel_workarounds.o \ > > gt/shmem_utils.o \ > > gt/sysfs_engines.o Raag
Hi Raag On 10/16/2024 2:31 PM, Raag Jadav wrote: > On Tue, Oct 15, 2024 at 03:56:10PM +0530, Riana Tauro wrote: >> Hi Raag >> >> On 10/11/2024 4:02 PM, Raag Jadav wrote: >>> Having similar naming convention in intel-family.h and intel_device_info.h >>> results in redefinition of a few platforms. Define CPU IDs in its own file >>> to avoid this. >>> >>> Signed-off-by: Raag Jadav <raag.jadav@intel.com> >>> --- >>> drivers/gpu/drm/i915/Makefile | 1 + >>> drivers/gpu/drm/i915/gt/intel_wa_cpu.c | 34 +++++++++++++++++++++ >>> drivers/gpu/drm/i915/gt/intel_workarounds.h | 2 ++ >>> 3 files changed, 37 insertions(+) >>> create mode 100644 drivers/gpu/drm/i915/gt/intel_wa_cpu.c >>> >>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile >>> index c63fa2133ccb..1f9b503ab976 100644 >>> --- a/drivers/gpu/drm/i915/Makefile >>> +++ b/drivers/gpu/drm/i915/Makefile >>> @@ -121,6 +121,7 @@ gt-y += \ >>> gt/intel_timeline.o \ >>> gt/intel_tlb.o \ >>> gt/intel_wopcm.o \ >>> + gt/intel_wa_cpu.o \ >> Why is this file under gt/ ? Doesn't seem to be gt specific > > The idea is to couple it with intel_workarounds.c This doesn't use any gt specific data. IMO should be out of gt, but upto you > Any other place it'd rather be? > >>> gt/intel_workarounds.o \ >>> gt/shmem_utils.o \ >>> gt/sysfs_engines.o > > Raag
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index c63fa2133ccb..1f9b503ab976 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -121,6 +121,7 @@ gt-y += \ gt/intel_timeline.o \ gt/intel_tlb.o \ gt/intel_wopcm.o \ + gt/intel_wa_cpu.o \ gt/intel_workarounds.o \ gt/shmem_utils.o \ gt/sysfs_engines.o diff --git a/drivers/gpu/drm/i915/gt/intel_wa_cpu.c b/drivers/gpu/drm/i915/gt/intel_wa_cpu.c new file mode 100644 index 000000000000..cbdab13e9db6 --- /dev/null +++ b/drivers/gpu/drm/i915/gt/intel_wa_cpu.c @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2024 Intel Corporation + * + * This file is introduced to avoid platform redefinition from + * intel_device_info.h :( + */ + +#include "intel_workarounds.h" + +#ifdef CONFIG_X86 +#include <asm/cpu_device_id.h> +#include <asm/intel-family.h> + +static const struct x86_cpu_id wa_cpu_ids[] = { + X86_MATCH_VFM(INTEL_ALDERLAKE, NULL), + X86_MATCH_VFM(INTEL_ALDERLAKE_L, NULL), + X86_MATCH_VFM(INTEL_COMETLAKE, NULL), + X86_MATCH_VFM(INTEL_KABYLAKE, NULL), + X86_MATCH_VFM(INTEL_KABYLAKE_L, NULL), + X86_MATCH_VFM(INTEL_RAPTORLAKE, NULL), + X86_MATCH_VFM(INTEL_RAPTORLAKE_P, NULL), + X86_MATCH_VFM(INTEL_RAPTORLAKE_S, NULL), + X86_MATCH_VFM(INTEL_ROCKETLAKE, NULL), + {} +}; + +bool intel_match_wa_cpu(void) +{ + return x86_match_cpu(wa_cpu_ids); +} +#else +bool intel_match_wa_cpu(void) { return false; } +#endif diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.h b/drivers/gpu/drm/i915/gt/intel_workarounds.h index 9beaab77c7f0..12f24fb31363 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.h +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.h @@ -21,6 +21,8 @@ static inline void intel_wa_list_free(struct i915_wa_list *wal) memset(wal, 0, sizeof(*wal)); } +bool intel_match_wa_cpu(void); + void intel_engine_init_ctx_wa(struct intel_engine_cs *engine); int intel_engine_emit_ctx_wa(struct i915_request *rq);
Having similar naming convention in intel-family.h and intel_device_info.h results in redefinition of a few platforms. Define CPU IDs in its own file to avoid this. Signed-off-by: Raag Jadav <raag.jadav@intel.com> --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gt/intel_wa_cpu.c | 34 +++++++++++++++++++++ drivers/gpu/drm/i915/gt/intel_workarounds.h | 2 ++ 3 files changed, 37 insertions(+) create mode 100644 drivers/gpu/drm/i915/gt/intel_wa_cpu.c