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

[Paloma Arellano]

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