Message ID | 20240125193834.7065-11-quic_parellan@quicinc.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Add support for CDM over DP | expand |
On 25/01/2024 21:38, Paloma Arellano wrote: > Modify dp_catalog_hw_revision to make the major and minor version values > known instead of outputting the entire hex value of the hardware version > register in preparation of using it for VSC SDP programming. > > Signed-off-by: Paloma Arellano <quic_parellan@quicinc.com> > --- > drivers/gpu/drm/msm/dp/dp_catalog.c | 12 +++++++++--- > drivers/gpu/drm/msm/dp/dp_catalog.h | 2 +- > 2 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c b/drivers/gpu/drm/msm/dp/dp_catalog.c > index 5d84c089e520a..c025786170ba5 100644 > --- a/drivers/gpu/drm/msm/dp/dp_catalog.c > +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c > @@ -24,6 +24,9 @@ > #define DP_INTERRUPT_STATUS_ACK_SHIFT 1 > #define DP_INTERRUPT_STATUS_MASK_SHIFT 2 > > +#define DP_HW_VERSION_MAJOR(reg) FIELD_GET(GENMASK(31, 28), reg) > +#define DP_HW_VERSION_MINOR(reg) FIELD_GET(GENMASK(27, 16), reg) > + > #define DP_INTF_CONFIG_DATABUS_WIDEN BIT(4) > > #define DP_INTERRUPT_STATUS1 \ > @@ -531,15 +534,18 @@ int dp_catalog_ctrl_set_pattern_state_bit(struct dp_catalog *dp_catalog, > * > * @dp_catalog: DP catalog structure > * > - * Return: DP controller hw revision > + * Return: void > * > */ > -u32 dp_catalog_hw_revision(const struct dp_catalog *dp_catalog) > +void dp_catalog_hw_revision(const struct dp_catalog *dp_catalog, u16 *major, u16 *minor) > { > const struct dp_catalog_private *catalog = container_of(dp_catalog, > struct dp_catalog_private, dp_catalog); > + u32 reg_dp_hw_version; > > - return dp_read_ahb(catalog, REG_DP_HW_VERSION); > + reg_dp_hw_version = dp_read_ahb(catalog, REG_DP_HW_VERSION); > + *major = DP_HW_VERSION_MAJOR(reg_dp_hw_version); > + *minor = DP_HW_VERSION_MINOR(reg_dp_hw_version); After looking at the code, it might be easier to keep dp_catalog_hw_revision as is, add define for hw revision 1.2 and corepare to it directly. > } > > /** > diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h b/drivers/gpu/drm/msm/dp/dp_catalog.h > index 563903605b3a7..94c377ef90c35 100644 > --- a/drivers/gpu/drm/msm/dp/dp_catalog.h > +++ b/drivers/gpu/drm/msm/dp/dp_catalog.h > @@ -170,7 +170,7 @@ void dp_catalog_ctrl_config_misc(struct dp_catalog *dp_catalog, u32 cc, u32 tb); > void dp_catalog_ctrl_config_msa(struct dp_catalog *dp_catalog, u32 rate, > u32 stream_rate_khz, bool fixed_nvid, bool is_ycbcr_420); > int dp_catalog_ctrl_set_pattern_state_bit(struct dp_catalog *dp_catalog, u32 pattern); > -u32 dp_catalog_hw_revision(const struct dp_catalog *dp_catalog); > +void dp_catalog_hw_revision(const struct dp_catalog *dp_catalog, u16 *major, u16 *minor); > void dp_catalog_ctrl_reset(struct dp_catalog *dp_catalog); > bool dp_catalog_ctrl_mainlink_ready(struct dp_catalog *dp_catalog); > void dp_catalog_ctrl_enable_irq(struct dp_catalog *dp_catalog, bool enable);
Hi Paloma, kernel test robot noticed the following build warnings: [auto build test WARNING on v6.8-rc1] [also build test WARNING on linus/master next-20240125] [cannot apply to drm-misc/drm-misc-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Paloma-Arellano/drm-msm-dpu-allow-dpu_encoder_helper_phys_setup_cdm-to-work-for-DP/20240126-034233 base: v6.8-rc1 patch link: https://lore.kernel.org/r/20240125193834.7065-11-quic_parellan%40quicinc.com patch subject: [PATCH 10/17] drm/msm/dp: modify dp_catalog_hw_revision to show major and minor val config: arm64-defconfig (https://download.01.org/0day-ci/archive/20240128/202401280752.AmrDI7Ox-lkp@intel.com/config) compiler: aarch64-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240128/202401280752.AmrDI7Ox-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202401280752.AmrDI7Ox-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/gpu/drm/msm/dp/dp_catalog.c:541: warning: Function parameter or struct member 'major' not described in 'dp_catalog_hw_revision' >> drivers/gpu/drm/msm/dp/dp_catalog.c:541: warning: Function parameter or struct member 'minor' not described in 'dp_catalog_hw_revision' vim +541 drivers/gpu/drm/msm/dp/dp_catalog.c c943b4948b5848 Chandan Uddaraju 2020-08-27 531 757a2f36ab095f Kuogee Hsieh 2022-02-25 532 /** 757a2f36ab095f Kuogee Hsieh 2022-02-25 533 * dp_catalog_hw_revision() - retrieve DP hw revision 757a2f36ab095f Kuogee Hsieh 2022-02-25 534 * 757a2f36ab095f Kuogee Hsieh 2022-02-25 535 * @dp_catalog: DP catalog structure 757a2f36ab095f Kuogee Hsieh 2022-02-25 536 * 5febc52d5716d6 Paloma Arellano 2024-01-25 537 * Return: void 757a2f36ab095f Kuogee Hsieh 2022-02-25 538 * 757a2f36ab095f Kuogee Hsieh 2022-02-25 539 */ 5febc52d5716d6 Paloma Arellano 2024-01-25 540 void dp_catalog_hw_revision(const struct dp_catalog *dp_catalog, u16 *major, u16 *minor) 757a2f36ab095f Kuogee Hsieh 2022-02-25 @541 { 757a2f36ab095f Kuogee Hsieh 2022-02-25 542 const struct dp_catalog_private *catalog = container_of(dp_catalog, 757a2f36ab095f Kuogee Hsieh 2022-02-25 543 struct dp_catalog_private, dp_catalog); 5febc52d5716d6 Paloma Arellano 2024-01-25 544 u32 reg_dp_hw_version; 757a2f36ab095f Kuogee Hsieh 2022-02-25 545 5febc52d5716d6 Paloma Arellano 2024-01-25 546 reg_dp_hw_version = dp_read_ahb(catalog, REG_DP_HW_VERSION); 5febc52d5716d6 Paloma Arellano 2024-01-25 547 *major = DP_HW_VERSION_MAJOR(reg_dp_hw_version); 5febc52d5716d6 Paloma Arellano 2024-01-25 548 *minor = DP_HW_VERSION_MINOR(reg_dp_hw_version); 757a2f36ab095f Kuogee Hsieh 2022-02-25 549 } 757a2f36ab095f Kuogee Hsieh 2022-02-25 550
On 1/25/2024 2:07 PM, Dmitry Baryshkov wrote: > On 25/01/2024 21:38, Paloma Arellano wrote: >> Modify dp_catalog_hw_revision to make the major and minor version values >> known instead of outputting the entire hex value of the hardware version >> register in preparation of using it for VSC SDP programming. >> >> Signed-off-by: Paloma Arellano <quic_parellan@quicinc.com> >> --- >> drivers/gpu/drm/msm/dp/dp_catalog.c | 12 +++++++++--- >> drivers/gpu/drm/msm/dp/dp_catalog.h | 2 +- >> 2 files changed, 10 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c >> b/drivers/gpu/drm/msm/dp/dp_catalog.c >> index 5d84c089e520a..c025786170ba5 100644 >> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c >> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c >> @@ -24,6 +24,9 @@ >> #define DP_INTERRUPT_STATUS_ACK_SHIFT 1 >> #define DP_INTERRUPT_STATUS_MASK_SHIFT 2 >> +#define DP_HW_VERSION_MAJOR(reg) FIELD_GET(GENMASK(31, 28), reg) >> +#define DP_HW_VERSION_MINOR(reg) FIELD_GET(GENMASK(27, 16), reg) >> + >> #define DP_INTF_CONFIG_DATABUS_WIDEN BIT(4) >> #define DP_INTERRUPT_STATUS1 \ >> @@ -531,15 +534,18 @@ int >> dp_catalog_ctrl_set_pattern_state_bit(struct dp_catalog *dp_catalog, >> * >> * @dp_catalog: DP catalog structure >> * >> - * Return: DP controller hw revision >> + * Return: void >> * >> */ >> -u32 dp_catalog_hw_revision(const struct dp_catalog *dp_catalog) >> +void dp_catalog_hw_revision(const struct dp_catalog *dp_catalog, u16 >> *major, u16 *minor) >> { >> const struct dp_catalog_private *catalog = >> container_of(dp_catalog, >> struct dp_catalog_private, dp_catalog); >> + u32 reg_dp_hw_version; >> - return dp_read_ahb(catalog, REG_DP_HW_VERSION); >> + reg_dp_hw_version = dp_read_ahb(catalog, REG_DP_HW_VERSION); >> + *major = DP_HW_VERSION_MAJOR(reg_dp_hw_version); >> + *minor = DP_HW_VERSION_MINOR(reg_dp_hw_version); > > After looking at the code, it might be easier to keep > dp_catalog_hw_revision as is, add define for hw revision 1.2 and > corepare to it directly. I thought having a define value of the version would be harder to follow than what's here currently. Since having it compare to the version value looks a little difficult to read versus having an explicit major and minor value version to compare to. For example having (major >= 1 && minor >= 2) versus having something like (hw_version >= DPU_HW_VERSION_1_2) > >> } >> /** >> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h >> b/drivers/gpu/drm/msm/dp/dp_catalog.h >> index 563903605b3a7..94c377ef90c35 100644 >> --- a/drivers/gpu/drm/msm/dp/dp_catalog.h >> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.h >> @@ -170,7 +170,7 @@ void dp_catalog_ctrl_config_misc(struct >> dp_catalog *dp_catalog, u32 cc, u32 tb); >> void dp_catalog_ctrl_config_msa(struct dp_catalog *dp_catalog, u32 >> rate, >> u32 stream_rate_khz, bool fixed_nvid, bool >> is_ycbcr_420); >> int dp_catalog_ctrl_set_pattern_state_bit(struct dp_catalog >> *dp_catalog, u32 pattern); >> -u32 dp_catalog_hw_revision(const struct dp_catalog *dp_catalog); >> +void dp_catalog_hw_revision(const struct dp_catalog *dp_catalog, u16 >> *major, u16 *minor); >> void dp_catalog_ctrl_reset(struct dp_catalog *dp_catalog); >> bool dp_catalog_ctrl_mainlink_ready(struct dp_catalog *dp_catalog); >> void dp_catalog_ctrl_enable_irq(struct dp_catalog *dp_catalog, bool >> enable); >
On Sun, 28 Jan 2024 at 07:31, Paloma Arellano <quic_parellan@quicinc.com> wrote: > > > On 1/25/2024 2:07 PM, Dmitry Baryshkov wrote: > > On 25/01/2024 21:38, Paloma Arellano wrote: > >> Modify dp_catalog_hw_revision to make the major and minor version values > >> known instead of outputting the entire hex value of the hardware version > >> register in preparation of using it for VSC SDP programming. > >> > >> Signed-off-by: Paloma Arellano <quic_parellan@quicinc.com> > >> --- > >> drivers/gpu/drm/msm/dp/dp_catalog.c | 12 +++++++++--- > >> drivers/gpu/drm/msm/dp/dp_catalog.h | 2 +- > >> 2 files changed, 10 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c > >> b/drivers/gpu/drm/msm/dp/dp_catalog.c > >> index 5d84c089e520a..c025786170ba5 100644 > >> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c > >> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c > >> @@ -24,6 +24,9 @@ > >> #define DP_INTERRUPT_STATUS_ACK_SHIFT 1 > >> #define DP_INTERRUPT_STATUS_MASK_SHIFT 2 > >> +#define DP_HW_VERSION_MAJOR(reg) FIELD_GET(GENMASK(31, 28), reg) > >> +#define DP_HW_VERSION_MINOR(reg) FIELD_GET(GENMASK(27, 16), reg) > >> + > >> #define DP_INTF_CONFIG_DATABUS_WIDEN BIT(4) > >> #define DP_INTERRUPT_STATUS1 \ > >> @@ -531,15 +534,18 @@ int > >> dp_catalog_ctrl_set_pattern_state_bit(struct dp_catalog *dp_catalog, > >> * > >> * @dp_catalog: DP catalog structure > >> * > >> - * Return: DP controller hw revision > >> + * Return: void > >> * > >> */ > >> -u32 dp_catalog_hw_revision(const struct dp_catalog *dp_catalog) > >> +void dp_catalog_hw_revision(const struct dp_catalog *dp_catalog, u16 > >> *major, u16 *minor) > >> { > >> const struct dp_catalog_private *catalog = > >> container_of(dp_catalog, > >> struct dp_catalog_private, dp_catalog); > >> + u32 reg_dp_hw_version; > >> - return dp_read_ahb(catalog, REG_DP_HW_VERSION); > >> + reg_dp_hw_version = dp_read_ahb(catalog, REG_DP_HW_VERSION); > >> + *major = DP_HW_VERSION_MAJOR(reg_dp_hw_version); > >> + *minor = DP_HW_VERSION_MINOR(reg_dp_hw_version); > > > > After looking at the code, it might be easier to keep > > dp_catalog_hw_revision as is, add define for hw revision 1.2 and > > corepare to it directly. > I thought having a define value of the version would be harder to > follow than what's here currently. Since having it compare to the > version value looks a little difficult to read versus having an explicit > major and minor value version to compare to. For example having (major > >= 1 && minor >= 2) versus having something like (hw_version >= > DPU_HW_VERSION_1_2) The problem is that major + minor are harder to follow and harder to implement. You got them wrong, btw. For example 2.1 is greater or equal than 1.2, but it doesn't pass your test. So, I think, a single define is easier and less error prone. > > > >> } > >> /** > >> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h > >> b/drivers/gpu/drm/msm/dp/dp_catalog.h > >> index 563903605b3a7..94c377ef90c35 100644 > >> --- a/drivers/gpu/drm/msm/dp/dp_catalog.h > >> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.h > >> @@ -170,7 +170,7 @@ void dp_catalog_ctrl_config_misc(struct > >> dp_catalog *dp_catalog, u32 cc, u32 tb); > >> void dp_catalog_ctrl_config_msa(struct dp_catalog *dp_catalog, u32 > >> rate, > >> u32 stream_rate_khz, bool fixed_nvid, bool > >> is_ycbcr_420); > >> int dp_catalog_ctrl_set_pattern_state_bit(struct dp_catalog > >> *dp_catalog, u32 pattern); > >> -u32 dp_catalog_hw_revision(const struct dp_catalog *dp_catalog); > >> +void dp_catalog_hw_revision(const struct dp_catalog *dp_catalog, u16 > >> *major, u16 *minor); > >> void dp_catalog_ctrl_reset(struct dp_catalog *dp_catalog); > >> bool dp_catalog_ctrl_mainlink_ready(struct dp_catalog *dp_catalog); > >> void dp_catalog_ctrl_enable_irq(struct dp_catalog *dp_catalog, bool > >> enable); > >
Hi Paloma, kernel test robot noticed the following build errors: [auto build test ERROR on v6.8-rc1] [also build test ERROR on linus/master next-20240125] [cannot apply to drm-misc/drm-misc-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Paloma-Arellano/drm-msm-dpu-allow-dpu_encoder_helper_phys_setup_cdm-to-work-for-DP/20240126-034233 base: v6.8-rc1 patch link: https://lore.kernel.org/r/20240125193834.7065-11-quic_parellan%40quicinc.com patch subject: [PATCH 10/17] drm/msm/dp: modify dp_catalog_hw_revision to show major and minor val config: arm-defconfig (https://download.01.org/0day-ci/archive/20240128/202401282131.j7UUVG6P-lkp@intel.com/config) compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240128/202401282131.j7UUVG6P-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202401282131.j7UUVG6P-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/gpu/drm/msm/dp/dp_catalog.c:547:11: error: implicit declaration of function 'FIELD_GET' is invalid in C99 [-Werror,-Wimplicit-function-declaration] *major = DP_HW_VERSION_MAJOR(reg_dp_hw_version); ^ drivers/gpu/drm/msm/dp/dp_catalog.c:27:34: note: expanded from macro 'DP_HW_VERSION_MAJOR' #define DP_HW_VERSION_MAJOR(reg) FIELD_GET(GENMASK(31, 28), reg) ^ 1 error generated. vim +/FIELD_GET +547 drivers/gpu/drm/msm/dp/dp_catalog.c 531 532 /** 533 * dp_catalog_hw_revision() - retrieve DP hw revision 534 * 535 * @dp_catalog: DP catalog structure 536 * 537 * Return: void 538 * 539 */ 540 void dp_catalog_hw_revision(const struct dp_catalog *dp_catalog, u16 *major, u16 *minor) 541 { 542 const struct dp_catalog_private *catalog = container_of(dp_catalog, 543 struct dp_catalog_private, dp_catalog); 544 u32 reg_dp_hw_version; 545 546 reg_dp_hw_version = dp_read_ahb(catalog, REG_DP_HW_VERSION); > 547 *major = DP_HW_VERSION_MAJOR(reg_dp_hw_version); 548 *minor = DP_HW_VERSION_MINOR(reg_dp_hw_version); 549 } 550
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c b/drivers/gpu/drm/msm/dp/dp_catalog.c index 5d84c089e520a..c025786170ba5 100644 --- a/drivers/gpu/drm/msm/dp/dp_catalog.c +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c @@ -24,6 +24,9 @@ #define DP_INTERRUPT_STATUS_ACK_SHIFT 1 #define DP_INTERRUPT_STATUS_MASK_SHIFT 2 +#define DP_HW_VERSION_MAJOR(reg) FIELD_GET(GENMASK(31, 28), reg) +#define DP_HW_VERSION_MINOR(reg) FIELD_GET(GENMASK(27, 16), reg) + #define DP_INTF_CONFIG_DATABUS_WIDEN BIT(4) #define DP_INTERRUPT_STATUS1 \ @@ -531,15 +534,18 @@ int dp_catalog_ctrl_set_pattern_state_bit(struct dp_catalog *dp_catalog, * * @dp_catalog: DP catalog structure * - * Return: DP controller hw revision + * Return: void * */ -u32 dp_catalog_hw_revision(const struct dp_catalog *dp_catalog) +void dp_catalog_hw_revision(const struct dp_catalog *dp_catalog, u16 *major, u16 *minor) { const struct dp_catalog_private *catalog = container_of(dp_catalog, struct dp_catalog_private, dp_catalog); + u32 reg_dp_hw_version; - return dp_read_ahb(catalog, REG_DP_HW_VERSION); + reg_dp_hw_version = dp_read_ahb(catalog, REG_DP_HW_VERSION); + *major = DP_HW_VERSION_MAJOR(reg_dp_hw_version); + *minor = DP_HW_VERSION_MINOR(reg_dp_hw_version); } /** diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h b/drivers/gpu/drm/msm/dp/dp_catalog.h index 563903605b3a7..94c377ef90c35 100644 --- a/drivers/gpu/drm/msm/dp/dp_catalog.h +++ b/drivers/gpu/drm/msm/dp/dp_catalog.h @@ -170,7 +170,7 @@ void dp_catalog_ctrl_config_misc(struct dp_catalog *dp_catalog, u32 cc, u32 tb); void dp_catalog_ctrl_config_msa(struct dp_catalog *dp_catalog, u32 rate, u32 stream_rate_khz, bool fixed_nvid, bool is_ycbcr_420); int dp_catalog_ctrl_set_pattern_state_bit(struct dp_catalog *dp_catalog, u32 pattern); -u32 dp_catalog_hw_revision(const struct dp_catalog *dp_catalog); +void dp_catalog_hw_revision(const struct dp_catalog *dp_catalog, u16 *major, u16 *minor); void dp_catalog_ctrl_reset(struct dp_catalog *dp_catalog); bool dp_catalog_ctrl_mainlink_ready(struct dp_catalog *dp_catalog); void dp_catalog_ctrl_enable_irq(struct dp_catalog *dp_catalog, bool enable);
Modify dp_catalog_hw_revision to make the major and minor version values known instead of outputting the entire hex value of the hardware version register in preparation of using it for VSC SDP programming. Signed-off-by: Paloma Arellano <quic_parellan@quicinc.com> --- drivers/gpu/drm/msm/dp/dp_catalog.c | 12 +++++++++--- drivers/gpu/drm/msm/dp/dp_catalog.h | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-)