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

[Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>]

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

--
With best wishes
Dmitry