| Message ID | 20240405-topic-smem_speedbin-v1-1-ce2b864251b1@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add SMEM-based speedbin matching | expand |
On Fri, Apr 05, 2024 at 10:41:29AM +0200, Konrad Dybcio wrote: > In preparation for parsing the chip "feature code" (FC) and "product > code" (PC) (essentially the parameters that let us conclusively > characterize the sillicon we're running on, including various speed > bins), move the socinfo version defines to the public header and > include some more FC/PC defines. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > drivers/soc/qcom/socinfo.c | 8 -------- > include/linux/soc/qcom/socinfo.h | 36 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 36 insertions(+), 8 deletions(-) > > diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c > index 277c07a6603d..cf4616a468f2 100644 > --- a/drivers/soc/qcom/socinfo.c > +++ b/drivers/soc/qcom/socinfo.c > @@ -21,14 +21,6 @@ > > #include <dt-bindings/arm/qcom,ids.h> > > -/* > - * SoC version type with major number in the upper 16 bits and minor > - * number in the lower 16 bits. > - */ > -#define SOCINFO_MAJOR(ver) (((ver) >> 16) & 0xffff) > -#define SOCINFO_MINOR(ver) ((ver) & 0xffff) > -#define SOCINFO_VERSION(maj, min) ((((maj) & 0xffff) << 16)|((min) & 0xffff)) > - > /* Helper macros to create soc_id table */ > #define qcom_board_id(id) QCOM_ID_ ## id, __stringify(id) > #define qcom_board_id_named(id, name) QCOM_ID_ ## id, (name) > diff --git a/include/linux/soc/qcom/socinfo.h b/include/linux/soc/qcom/socinfo.h > index e78777bb0f4a..ba7f683bd32c 100644 > --- a/include/linux/soc/qcom/socinfo.h > +++ b/include/linux/soc/qcom/socinfo.h > @@ -3,6 +3,8 @@ > #ifndef __QCOM_SOCINFO_H__ > #define __QCOM_SOCINFO_H__ > > +#include <linux/types.h> > + > /* > * SMEM item id, used to acquire handles to respective > * SMEM region. > @@ -12,6 +14,14 @@ > #define SMEM_SOCINFO_BUILD_ID_LENGTH 32 > #define SMEM_SOCINFO_CHIP_ID_LENGTH 32 > > +/* > + * SoC version type with major number in the upper 16 bits and minor > + * number in the lower 16 bits. > + */ > +#define SOCINFO_MAJOR(ver) (((ver) >> 16) & 0xffff) > +#define SOCINFO_MINOR(ver) ((ver) & 0xffff) > +#define SOCINFO_VERSION(maj, min) ((((maj) & 0xffff) << 16)|((min) & 0xffff)) > + > /* Socinfo SMEM item structure */ > struct socinfo { > __le32 fmt; > @@ -74,4 +84,30 @@ struct socinfo { > __le32 boot_core; > }; > > +/* Internal feature codes */ > +enum feature_code { > + /* External feature codes */ > + SOCINFO_FC_UNKNOWN = 0x0, > + SOCINFO_FC_AA, > + SOCINFO_FC_AB, > + SOCINFO_FC_AC, > + SOCINFO_FC_AD, > + SOCINFO_FC_AE, > + SOCINFO_FC_AF, > + SOCINFO_FC_AG, > + SOCINFO_FC_AH, > + SOCINFO_FC_EXT_RESERVE, > +}; > + > +/* Internal feature codes */ > +/* Valid values: 0 <= n <= 0xf */ > +#define SOCINFO_FC_Yn(n) (0xf1 + n) > +#define SOCINFO_FC_INT_RESERVE SOCINFO_FC_Yn(0x10) > + > +/* Product codes */ > +#define SOCINFO_PC_UNKNOWN 0 > +/* Valid values: 0 <= n <= 8, the rest is reserved */ > +#define SOCINFO_PCn(n) (n + 1) > +#define SOCINFO_PC_RESERVE (BIT(31) - 1) Please move these defines into the next patch. > + > #endif > > -- > 2.40.1 >
On Fri, Apr 05, 2024 at 10:41:29AM +0200, Konrad Dybcio wrote: > In preparation for parsing the chip "feature code" (FC) and "product > code" (PC) (essentially the parameters that let us conclusively > characterize the sillicon we're running on, including various speed > bins), move the socinfo version defines to the public header and > include some more FC/PC defines. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > drivers/soc/qcom/socinfo.c | 8 -------- > include/linux/soc/qcom/socinfo.h | 36 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 36 insertions(+), 8 deletions(-) > ... > diff --git a/include/linux/soc/qcom/socinfo.h b/include/linux/soc/qcom/socinfo.h ... > @@ -74,4 +84,30 @@ struct socinfo { > __le32 boot_core; > }; > > +/* Internal feature codes */ > +enum feature_code { > + /* External feature codes */ > + SOCINFO_FC_UNKNOWN = 0x0, > + SOCINFO_FC_AA, > + SOCINFO_FC_AB, > + SOCINFO_FC_AC, > + SOCINFO_FC_AD, > + SOCINFO_FC_AE, > + SOCINFO_FC_AF, > + SOCINFO_FC_AG, > + SOCINFO_FC_AH, > + SOCINFO_FC_EXT_RESERVE, > +}; SOCINFO_FC_EXT_RESERVE was a convenient limit since we mapped SOCINFO_FC_AA -> string "AA" via an array, and we've only needed the 8 feature codes so far. We should remove the EXT_RESERVE and test for the Y0-YF (internal feature code) values instead. > + > +/* Internal feature codes */ > +/* Valid values: 0 <= n <= 0xf */ > +#define SOCINFO_FC_Yn(n) (0xf1 + n) > +#define SOCINFO_FC_INT_RESERVE SOCINFO_FC_Yn(0x10) We probably should've named this SOCINFO_FC_INT_MAX. Reserve implies it's reserved for some future use, but it's really the max value it could be. > + > +/* Product codes */ > +#define SOCINFO_PC_UNKNOWN 0 > +/* Valid values: 0 <= n <= 8, the rest is reserved */ > +#define SOCINFO_PCn(n) (n + 1) > +#define SOCINFO_PC_RESERVE (BIT(31) - 1) Similar comments here as the SOCINFO_FC_EXT_*. It's more like known values are [0,8], but more values could come in future chipsets. > + > #endif >
On 4/11/24 20:55, Elliot Berman wrote: > On Fri, Apr 05, 2024 at 10:41:29AM +0200, Konrad Dybcio wrote: >> In preparation for parsing the chip "feature code" (FC) and "product >> code" (PC) (essentially the parameters that let us conclusively >> characterize the sillicon we're running on, including various speed >> bins), move the socinfo version defines to the public header and >> include some more FC/PC defines. >> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> --- [...] >> + SOCINFO_FC_EXT_RESERVE, >> +}; > > SOCINFO_FC_EXT_RESERVE was a convenient limit since we mapped > SOCINFO_FC_AA -> string "AA" via an array, and we've only needed the 8 > feature codes so far. > > We should remove the EXT_RESERVE and test for the Y0-YF (internal > feature code) values instead. OK > >> + >> +/* Internal feature codes */ >> +/* Valid values: 0 <= n <= 0xf */ >> +#define SOCINFO_FC_Yn(n) (0xf1 + n) >> +#define SOCINFO_FC_INT_RESERVE SOCINFO_FC_Yn(0x10) > > We probably should've named this SOCINFO_FC_INT_MAX. Reserve implies > it's reserved for some future use, but it's really the max value it > could be. So, should SOCINFO_FC_Yn(0x10) also be considered valid, or is (0xf) the last one? > >> + >> +/* Product codes */ >> +#define SOCINFO_PC_UNKNOWN 0 >> +/* Valid values: 0 <= n <= 8, the rest is reserved */ >> +#define SOCINFO_PCn(n) (n + 1) >> +#define SOCINFO_PC_RESERVE (BIT(31) - 1) > > Similar comments here as the SOCINFO_FC_EXT_*. It's more like known > values are [0,8], but more values could come in future chipsets. Ok, sounds good, I'll remove the comment then Konrad
On Thu, Apr 11, 2024 at 10:05:30PM +0200, Konrad Dybcio wrote: > > > On 4/11/24 20:55, Elliot Berman wrote: > > On Fri, Apr 05, 2024 at 10:41:29AM +0200, Konrad Dybcio wrote: > > > In preparation for parsing the chip "feature code" (FC) and "product > > > code" (PC) (essentially the parameters that let us conclusively > > > characterize the sillicon we're running on, including various speed > > > bins), move the socinfo version defines to the public header and > > > include some more FC/PC defines. > > > > > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > > > --- > > [...] > > > > + SOCINFO_FC_EXT_RESERVE, > > > +}; > > > > SOCINFO_FC_EXT_RESERVE was a convenient limit since we mapped > > SOCINFO_FC_AA -> string "AA" via an array, and we've only needed the 8 > > feature codes so far. > > > > We should remove the EXT_RESERVE and test for the Y0-YF (internal > > feature code) values instead. > > OK > > > > > > + > > > +/* Internal feature codes */ > > > +/* Valid values: 0 <= n <= 0xf */ > > > +#define SOCINFO_FC_Yn(n) (0xf1 + n) > > > +#define SOCINFO_FC_INT_RESERVE SOCINFO_FC_Yn(0x10) > > > > We probably should've named this SOCINFO_FC_INT_MAX. Reserve implies > > it's reserved for some future use, but it's really the max value it > > could be. > > So, should SOCINFO_FC_Yn(0x10) also be considered valid, or is (0xf) > the last one? > 0xf is the last one. Thanks, Elliot > > > > > + > > > +/* Product codes */ > > > +#define SOCINFO_PC_UNKNOWN 0 > > > +/* Valid values: 0 <= n <= 8, the rest is reserved */ > > > +#define SOCINFO_PCn(n) (n + 1) > > > +#define SOCINFO_PC_RESERVE (BIT(31) - 1) > > > > Similar comments here as the SOCINFO_FC_EXT_*. It's more like known > > values are [0,8], but more values could come in future chipsets. > > Ok, sounds good, I'll remove the comment then > > Konrad
On 4/11/24 22:09, Elliot Berman wrote: > On Thu, Apr 11, 2024 at 10:05:30PM +0200, Konrad Dybcio wrote: >> >> >> On 4/11/24 20:55, Elliot Berman wrote: >>> On Fri, Apr 05, 2024 at 10:41:29AM +0200, Konrad Dybcio wrote: >>>> In preparation for parsing the chip "feature code" (FC) and "product >>>> code" (PC) (essentially the parameters that let us conclusively >>>> characterize the sillicon we're running on, including various speed >>>> bins), move the socinfo version defines to the public header and >>>> include some more FC/PC defines. >>>> >>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >>>> --- [...] > > 0xf is the last one. One more question, are the "internal/external feature codes" referring to internality/externality of the chips (i.e. "are they QC-lab-only engineering samples), or what else does that represent? Konrad
On Thu, Apr 11, 2024 at 10:24:08PM +0200, Konrad Dybcio wrote: > > > On 4/11/24 22:09, Elliot Berman wrote: > > On Thu, Apr 11, 2024 at 10:05:30PM +0200, Konrad Dybcio wrote: > > > > > > > > > On 4/11/24 20:55, Elliot Berman wrote: > > > > On Fri, Apr 05, 2024 at 10:41:29AM +0200, Konrad Dybcio wrote: > > > > > In preparation for parsing the chip "feature code" (FC) and "product > > > > > code" (PC) (essentially the parameters that let us conclusively > > > > > characterize the sillicon we're running on, including various speed > > > > > bins), move the socinfo version defines to the public header and > > > > > include some more FC/PC defines. > > > > > > > > > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > > > > > --- > > [...] > > > > > 0xf is the last one. > > One more question, are the "internal/external feature codes" referring to > internality/externality of the chips (i.e. "are they QC-lab-only engineering > samples), or what else does that represent? Yes, QC-lab-only engineering samples is the right interpretation of these feature codes. Thanks, Elliot
On 4/12/24 01:49, Elliot Berman wrote: > On Thu, Apr 11, 2024 at 10:24:08PM +0200, Konrad Dybcio wrote: >> >> >> On 4/11/24 22:09, Elliot Berman wrote: >>> On Thu, Apr 11, 2024 at 10:05:30PM +0200, Konrad Dybcio wrote: >>>> >>>> >>>> On 4/11/24 20:55, Elliot Berman wrote: >>>>> On Fri, Apr 05, 2024 at 10:41:29AM +0200, Konrad Dybcio wrote: >>>>>> In preparation for parsing the chip "feature code" (FC) and "product >>>>>> code" (PC) (essentially the parameters that let us conclusively >>>>>> characterize the sillicon we're running on, including various speed >>>>>> bins), move the socinfo version defines to the public header and >>>>>> include some more FC/PC defines. >>>>>> >>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >>>>>> --- >> >> [...] >> >>> >>> 0xf is the last one. >> >> One more question, are the "internal/external feature codes" referring to >> internality/externality of the chips (i.e. "are they QC-lab-only engineering >> samples), or what else does that represent? > > Yes, QC-lab-only engineering samples is the right interpretation of > these feature codes. Do you think it would be beneficial to keep the logic for these ESes in the upstream GPU driver? Otherwise, I can yank out half of the added lines. Konrad
On Fri, Apr 12, 2024 at 02:10:30AM +0200, Konrad Dybcio wrote: > > > On 4/12/24 01:49, Elliot Berman wrote: > > On Thu, Apr 11, 2024 at 10:24:08PM +0200, Konrad Dybcio wrote: > > > > > > > > > On 4/11/24 22:09, Elliot Berman wrote: > > > > On Thu, Apr 11, 2024 at 10:05:30PM +0200, Konrad Dybcio wrote: > > > > > > > > > > > > > > > On 4/11/24 20:55, Elliot Berman wrote: > > > > > > On Fri, Apr 05, 2024 at 10:41:29AM +0200, Konrad Dybcio wrote: > > > > > > > In preparation for parsing the chip "feature code" (FC) and "product > > > > > > > code" (PC) (essentially the parameters that let us conclusively > > > > > > > characterize the sillicon we're running on, including various speed > > > > > > > bins), move the socinfo version defines to the public header and > > > > > > > include some more FC/PC defines. > > > > > > > > > > > > > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > > > > > > > --- > > > > > > [...] > > > > > > > > > > > 0xf is the last one. > > > > > > One more question, are the "internal/external feature codes" referring to > > > internality/externality of the chips (i.e. "are they QC-lab-only engineering > > > samples), or what else does that represent? > > > > Yes, QC-lab-only engineering samples is the right interpretation of > > these feature codes. > > Do you think it would be beneficial to keep the logic for these ESes in > the upstream GPU driver? Otherwise, I can yank out half of the added lines. > Should be fine to yank, IMO.
diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c index 277c07a6603d..cf4616a468f2 100644 --- a/drivers/soc/qcom/socinfo.c +++ b/drivers/soc/qcom/socinfo.c @@ -21,14 +21,6 @@ #include <dt-bindings/arm/qcom,ids.h> -/* - * SoC version type with major number in the upper 16 bits and minor - * number in the lower 16 bits. - */ -#define SOCINFO_MAJOR(ver) (((ver) >> 16) & 0xffff) -#define SOCINFO_MINOR(ver) ((ver) & 0xffff) -#define SOCINFO_VERSION(maj, min) ((((maj) & 0xffff) << 16)|((min) & 0xffff)) - /* Helper macros to create soc_id table */ #define qcom_board_id(id) QCOM_ID_ ## id, __stringify(id) #define qcom_board_id_named(id, name) QCOM_ID_ ## id, (name) diff --git a/include/linux/soc/qcom/socinfo.h b/include/linux/soc/qcom/socinfo.h index e78777bb0f4a..ba7f683bd32c 100644 --- a/include/linux/soc/qcom/socinfo.h +++ b/include/linux/soc/qcom/socinfo.h @@ -3,6 +3,8 @@ #ifndef __QCOM_SOCINFO_H__ #define __QCOM_SOCINFO_H__ +#include <linux/types.h> + /* * SMEM item id, used to acquire handles to respective * SMEM region. @@ -12,6 +14,14 @@ #define SMEM_SOCINFO_BUILD_ID_LENGTH 32 #define SMEM_SOCINFO_CHIP_ID_LENGTH 32 +/* + * SoC version type with major number in the upper 16 bits and minor + * number in the lower 16 bits. + */ +#define SOCINFO_MAJOR(ver) (((ver) >> 16) & 0xffff) +#define SOCINFO_MINOR(ver) ((ver) & 0xffff) +#define SOCINFO_VERSION(maj, min) ((((maj) & 0xffff) << 16)|((min) & 0xffff)) + /* Socinfo SMEM item structure */ struct socinfo { __le32 fmt; @@ -74,4 +84,30 @@ struct socinfo { __le32 boot_core; }; +/* Internal feature codes */ +enum feature_code { + /* External feature codes */ + SOCINFO_FC_UNKNOWN = 0x0, + SOCINFO_FC_AA, + SOCINFO_FC_AB, + SOCINFO_FC_AC, + SOCINFO_FC_AD, + SOCINFO_FC_AE, + SOCINFO_FC_AF, + SOCINFO_FC_AG, + SOCINFO_FC_AH, + SOCINFO_FC_EXT_RESERVE, +}; + +/* Internal feature codes */ +/* Valid values: 0 <= n <= 0xf */ +#define SOCINFO_FC_Yn(n) (0xf1 + n) +#define SOCINFO_FC_INT_RESERVE SOCINFO_FC_Yn(0x10) + +/* Product codes */ +#define SOCINFO_PC_UNKNOWN 0 +/* Valid values: 0 <= n <= 8, the rest is reserved */ +#define SOCINFO_PCn(n) (n + 1) +#define SOCINFO_PC_RESERVE (BIT(31) - 1) + #endif
In preparation for parsing the chip "feature code" (FC) and "product code" (PC) (essentially the parameters that let us conclusively characterize the sillicon we're running on, including various speed bins), move the socinfo version defines to the public header and include some more FC/PC defines. Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- drivers/soc/qcom/socinfo.c | 8 -------- include/linux/soc/qcom/socinfo.h | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 8 deletions(-)