diff mbox series

[10/17] drm/msm/dp: modify dp_catalog_hw_revision to show major and minor val

Message ID 20240125193834.7065-11-quic_parellan@quicinc.com
State New
Headers show
Series Add support for CDM over DP | expand

Commit Message

Paloma Arellano Jan. 25, 2024, 7:38 p.m. UTC
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(-)

Comments

kernel test robot Jan. 27, 2024, 11:43 p.m. UTC | #1
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
Paloma Arellano Jan. 28, 2024, 5:30 a.m. UTC | #2
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);
>
kernel test robot Jan. 28, 2024, 2:02 p.m. UTC | #3
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 mbox series

Patch

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);