Re: [PATCH 2/3] drm/msm/dpu: Add MISR register support for interface

[Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>]

On 27/05/2022 21:54, Jessica Zhang wrote:
> Add support for setting MISR registers within the interface
>
> Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 55 ++++++++++++++++++++-
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h |  8 ++-
>   2 files changed, 61 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> index 3f4d2c6e1b45..29aaeff9eacd 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> @@ -1,5 +1,7 @@
>   // SPDX-License-Identifier: GPL-2.0-only
> -/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
> +/*
> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
>    */
>   
>   #include "dpu_hwio.h"
> @@ -67,6 +69,14 @@
>   #define INTF_CFG2_DATABUS_WIDEN	BIT(0)
>   #define INTF_CFG2_DATA_HCTL_EN	BIT(4)
>   
> +#define INTF_MISR_CTRL			0x180
> +#define INTF_MISR_SIGNATURE		0x184
> +#define INTF_MISR_FRAME_COUNT_MASK	0xFF
> +#define INTF_MISR_CTRL_ENABLE		BIT(8)
> +#define INTF_MISR_CTRL_STATUS		BIT(9)
> +#define INTF_MISR_CTRL_STATUS_CLEAR	BIT(10)
> +#define INTF_MISR_CTRL_FREE_RUN_MASK	BIT(31)

I'm tempted to ask to move these bits to some common header. Is there 
any other hardware block which uses the same bitfields to control MISR?

> +
>   static const struct dpu_intf_cfg *_intf_offset(enum dpu_intf intf,
>   		const struct dpu_mdss_cfg *m,
>   		void __iomem *addr,
> @@ -319,6 +329,47 @@ static u32 dpu_hw_intf_get_line_count(struct dpu_hw_intf *intf)
>   	return DPU_REG_READ(c, INTF_LINE_COUNT);
>   }
>   
> +static void dpu_hw_intf_setup_misr(struct dpu_hw_intf *intf, bool enable, u32 frame_count)
> +{
> +	struct dpu_hw_blk_reg_map *c = &intf->hw;
> +	u32 config = 0;
> +
> +	DPU_REG_WRITE(c, INTF_MISR_CTRL, INTF_MISR_CTRL_STATUS_CLEAR);
> +
> +	/* Clear old MISR value (in case it's read before a new value is calculated)*/
> +	wmb();
> +
> +	if (enable) {
> +		config = (frame_count & INTF_MISR_FRAME_COUNT_MASK) |
> +				INTF_MISR_CTRL_ENABLE | INTF_MISR_CTRL_FREE_RUN_MASK;
> +
> +		DPU_REG_WRITE(c, INTF_MISR_CTRL, config);
> +	} else {
> +		DPU_REG_WRITE(c, INTF_MISR_CTRL, 0);
> +	}
> +}
> +
> +static int dpu_hw_intf_collect_misr(struct dpu_hw_intf *intf, u32 *misr_value)
> +{
> +	struct dpu_hw_blk_reg_map *c = &intf->hw;
> +	u32 ctrl = 0;
> +
> +	if (!misr_value)
> +		return -EINVAL;
> +
> +	ctrl = DPU_REG_READ(c, INTF_MISR_CTRL);
> +
> +	if (!(ctrl & INTF_MISR_CTRL_ENABLE))
> +		return -ENODATA;
> +
> +	if (!(ctrl & INTF_MISR_CTRL_STATUS))
> +		return -EINVAL;
> +
> +	*misr_value = DPU_REG_READ(c, INTF_MISR_SIGNATURE);
> +
> +	return 0;
> +}
> +
>   static void _setup_intf_ops(struct dpu_hw_intf_ops *ops,
>   		unsigned long cap)
>   {
> @@ -329,6 +380,8 @@ static void _setup_intf_ops(struct dpu_hw_intf_ops *ops,
>   	ops->get_line_count = dpu_hw_intf_get_line_count;
>   	if (cap & BIT(DPU_INTF_INPUT_CTRL))
>   		ops->bind_pingpong_blk = dpu_hw_intf_bind_pingpong_blk;
> +	ops->setup_misr = dpu_hw_intf_setup_misr;
> +	ops->collect_misr = dpu_hw_intf_collect_misr;
>   }
>   
>   struct dpu_hw_intf *dpu_hw_intf_init(enum dpu_intf idx,
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> index 7b2d96ac61e8..8d0e7b509260 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> @@ -1,5 +1,7 @@
>   /* SPDX-License-Identifier: GPL-2.0-only */
> -/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
> +/*
> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
>    */
>   
>   #ifndef _DPU_HW_INTF_H
> @@ -57,6 +59,8 @@ struct intf_status {
>    * @ get_line_count: reads current vertical line counter
>    * @bind_pingpong_blk: enable/disable the connection with pingpong which will
>    *                     feed pixels to this interface
> + * @setup_misr: enable/disable MISR
> + * @collect_misr: read MISR signature
>    */
>   struct dpu_hw_intf_ops {
>   	void (*setup_timing_gen)(struct dpu_hw_intf *intf,
> @@ -77,6 +81,8 @@ struct dpu_hw_intf_ops {
>   	void (*bind_pingpong_blk)(struct dpu_hw_intf *intf,
>   			bool enable,
>   			const enum dpu_pingpong pp);
> +	void (*setup_misr)(struct dpu_hw_intf *intf, bool enable, u32 frame_count);
> +	int (*collect_misr)(struct dpu_hw_intf *intf, u32 *misr_value);
>   };
>   
>   struct dpu_hw_intf {


--
With best wishes
Dmitry