diff mbox series

[2/7] drm/msm/dpu: convert vsync source defines to the enum

Message ID 20240520-dpu-handle-te-signal-v1-2-f273b42a089c@linaro.org (mailing list archive)
State New, archived
Headers show
Series drm/msm/dpu: handle non-default TE source pins | expand

Commit Message

Dmitry Baryshkov May 20, 2024, 12:12 p.m. UTC 
Add enum dpu_vsync_source instead of a series of defines. Use this enum
to pass vsync information.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c |  2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h |  2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 26 ++++++++++++++------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h  |  2 +-
 5 files changed, 18 insertions(+), 16 deletions(-)

Comments

Abhinav Kumar May 22, 2024, 6:41 p.m. UTC | #1 
On 5/20/2024 5:12 AM, Dmitry Baryshkov wrote:
> Add enum dpu_vsync_source instead of a series of defines. Use this enum
> to pass vsync information.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  2 +-
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c |  2 +-
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h |  2 +-
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 26 ++++++++++++++------------
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h  |  2 +-
>   5 files changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 119f3ea50a7c..4988a1029431 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -747,7 +747,7 @@ static void _dpu_encoder_update_vsync_source(struct dpu_encoder_virt *dpu_enc,
>   		if (disp_info->is_te_using_watchdog_timer)
>   			vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_WD_TIMER_0;
>   		else
> -			vsync_cfg.vsync_source = DPU_VSYNC0_SOURCE_GPIO;
> +			vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
>   
>   		hw_mdptop->ops.setup_vsync_source(hw_mdptop, &vsync_cfg);
>   
> 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 225c1c7768ff..96f6160cf607 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> @@ -462,7 +462,7 @@ static int dpu_hw_intf_get_vsync_info(struct dpu_hw_intf *intf,
>   }
>   
>   static void dpu_hw_intf_vsync_sel(struct dpu_hw_intf *intf,
> -		u32 vsync_source)
> +				  enum dpu_vsync_source vsync_source)
>   {
>   	struct dpu_hw_blk_reg_map *c;
>   
> 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 f9015c67a574..ac244f0b33fb 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> @@ -107,7 +107,7 @@ struct dpu_hw_intf_ops {
>   
>   	int (*connect_external_te)(struct dpu_hw_intf *intf, bool enable_external_te);
>   
> -	void (*vsync_sel)(struct dpu_hw_intf *intf, u32 vsync_source);
> +	void (*vsync_sel)(struct dpu_hw_intf *intf, enum dpu_vsync_source vsync_source);
>   
>   	/**
>   	 * Disable autorefresh if enabled
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> index 66759623fc42..a2eff36a2224 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> @@ -54,18 +54,20 @@
>   #define DPU_BLEND_BG_INV_MOD_ALPHA	(1 << 12)
>   #define DPU_BLEND_BG_TRANSP_EN		(1 << 13)
>   
> -#define DPU_VSYNC0_SOURCE_GPIO		0
> -#define DPU_VSYNC1_SOURCE_GPIO		1
> -#define DPU_VSYNC2_SOURCE_GPIO		2
> -#define DPU_VSYNC_SOURCE_INTF_0		3
> -#define DPU_VSYNC_SOURCE_INTF_1		4
> -#define DPU_VSYNC_SOURCE_INTF_2		5
> -#define DPU_VSYNC_SOURCE_INTF_3		6
> -#define DPU_VSYNC_SOURCE_WD_TIMER_4	11
> -#define DPU_VSYNC_SOURCE_WD_TIMER_3	12
> -#define DPU_VSYNC_SOURCE_WD_TIMER_2	13
> -#define DPU_VSYNC_SOURCE_WD_TIMER_1	14
> -#define DPU_VSYNC_SOURCE_WD_TIMER_0	15
> +enum dpu_vsync_source {
> +	DPU_VSYNC_SOURCE_GPIO_0,
> +	DPU_VSYNC_SOURCE_GPIO_1,
> +	DPU_VSYNC_SOURCE_GPIO_2,
> +	DPU_VSYNC_SOURCE_INTF_0 = 3,

Do we need this assignment to 3?

Rest LGTM,

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Dmitry Baryshkov May 22, 2024, 8:01 p.m. UTC | #2 
On Wed, 22 May 2024 at 21:41, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>
>
>
> On 5/20/2024 5:12 AM, Dmitry Baryshkov wrote:
> > Add enum dpu_vsync_source instead of a series of defines. Use this enum
> > to pass vsync information.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  2 +-
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c |  2 +-
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h |  2 +-
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 26 ++++++++++++++------------
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h  |  2 +-
> >   5 files changed, 18 insertions(+), 16 deletions(-)
> >

> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > index 66759623fc42..a2eff36a2224 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > @@ -54,18 +54,20 @@
> >   #define DPU_BLEND_BG_INV_MOD_ALPHA  (1 << 12)
> >   #define DPU_BLEND_BG_TRANSP_EN              (1 << 13)
> >
> > -#define DPU_VSYNC0_SOURCE_GPIO               0
> > -#define DPU_VSYNC1_SOURCE_GPIO               1
> > -#define DPU_VSYNC2_SOURCE_GPIO               2
> > -#define DPU_VSYNC_SOURCE_INTF_0              3
> > -#define DPU_VSYNC_SOURCE_INTF_1              4
> > -#define DPU_VSYNC_SOURCE_INTF_2              5
> > -#define DPU_VSYNC_SOURCE_INTF_3              6
> > -#define DPU_VSYNC_SOURCE_WD_TIMER_4  11
> > -#define DPU_VSYNC_SOURCE_WD_TIMER_3  12
> > -#define DPU_VSYNC_SOURCE_WD_TIMER_2  13
> > -#define DPU_VSYNC_SOURCE_WD_TIMER_1  14
> > -#define DPU_VSYNC_SOURCE_WD_TIMER_0  15
> > +enum dpu_vsync_source {
> > +     DPU_VSYNC_SOURCE_GPIO_0,
> > +     DPU_VSYNC_SOURCE_GPIO_1,
> > +     DPU_VSYNC_SOURCE_GPIO_2,
> > +     DPU_VSYNC_SOURCE_INTF_0 = 3,
>
> Do we need this assignment to 3?

It is redundant, but it points out that if at some point another GPIO
is added, it should go to the end (or to some other place, having the
proper value).

>
> Rest LGTM,
>
> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Abhinav Kumar May 22, 2024, 11:57 p.m. UTC | #3 
On 5/22/2024 1:01 PM, Dmitry Baryshkov wrote:
> On Wed, 22 May 2024 at 21:41, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>>
>>
>>
>> On 5/20/2024 5:12 AM, Dmitry Baryshkov wrote:
>>> Add enum dpu_vsync_source instead of a series of defines. Use this enum
>>> to pass vsync information.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  2 +-
>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c |  2 +-
>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h |  2 +-
>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 26 ++++++++++++++------------
>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h  |  2 +-
>>>    5 files changed, 18 insertions(+), 16 deletions(-)
>>>
> 
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
>>> index 66759623fc42..a2eff36a2224 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
>>> @@ -54,18 +54,20 @@
>>>    #define DPU_BLEND_BG_INV_MOD_ALPHA  (1 << 12)
>>>    #define DPU_BLEND_BG_TRANSP_EN              (1 << 13)
>>>
>>> -#define DPU_VSYNC0_SOURCE_GPIO               0
>>> -#define DPU_VSYNC1_SOURCE_GPIO               1
>>> -#define DPU_VSYNC2_SOURCE_GPIO               2
>>> -#define DPU_VSYNC_SOURCE_INTF_0              3
>>> -#define DPU_VSYNC_SOURCE_INTF_1              4
>>> -#define DPU_VSYNC_SOURCE_INTF_2              5
>>> -#define DPU_VSYNC_SOURCE_INTF_3              6
>>> -#define DPU_VSYNC_SOURCE_WD_TIMER_4  11
>>> -#define DPU_VSYNC_SOURCE_WD_TIMER_3  12
>>> -#define DPU_VSYNC_SOURCE_WD_TIMER_2  13
>>> -#define DPU_VSYNC_SOURCE_WD_TIMER_1  14
>>> -#define DPU_VSYNC_SOURCE_WD_TIMER_0  15
>>> +enum dpu_vsync_source {
>>> +     DPU_VSYNC_SOURCE_GPIO_0,
>>> +     DPU_VSYNC_SOURCE_GPIO_1,
>>> +     DPU_VSYNC_SOURCE_GPIO_2,
>>> +     DPU_VSYNC_SOURCE_INTF_0 = 3,
>>
>> Do we need this assignment to 3?
> 
> It is redundant, but it points out that if at some point another GPIO
> is added, it should go to the end (or to some other place, having the
> proper value).
> 

Ack, makes sense.

>>
>> Rest LGTM,
>>
>> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 119f3ea50a7c..4988a1029431 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -747,7 +747,7 @@  static void _dpu_encoder_update_vsync_source(struct dpu_encoder_virt *dpu_enc,
 		if (disp_info->is_te_using_watchdog_timer)
 			vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_WD_TIMER_0;
 		else
-			vsync_cfg.vsync_source = DPU_VSYNC0_SOURCE_GPIO;
+			vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
 
 		hw_mdptop->ops.setup_vsync_source(hw_mdptop, &vsync_cfg);
 
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 225c1c7768ff..96f6160cf607 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -462,7 +462,7 @@  static int dpu_hw_intf_get_vsync_info(struct dpu_hw_intf *intf,
 }
 
 static void dpu_hw_intf_vsync_sel(struct dpu_hw_intf *intf,
-		u32 vsync_source)
+				  enum dpu_vsync_source vsync_source)
 {
 	struct dpu_hw_blk_reg_map *c;
 
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 f9015c67a574..ac244f0b33fb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
@@ -107,7 +107,7 @@  struct dpu_hw_intf_ops {
 
 	int (*connect_external_te)(struct dpu_hw_intf *intf, bool enable_external_te);
 
-	void (*vsync_sel)(struct dpu_hw_intf *intf, u32 vsync_source);
+	void (*vsync_sel)(struct dpu_hw_intf *intf, enum dpu_vsync_source vsync_source);
 
 	/**
 	 * Disable autorefresh if enabled
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
index 66759623fc42..a2eff36a2224 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -54,18 +54,20 @@ 
 #define DPU_BLEND_BG_INV_MOD_ALPHA	(1 << 12)
 #define DPU_BLEND_BG_TRANSP_EN		(1 << 13)
 
-#define DPU_VSYNC0_SOURCE_GPIO		0
-#define DPU_VSYNC1_SOURCE_GPIO		1
-#define DPU_VSYNC2_SOURCE_GPIO		2
-#define DPU_VSYNC_SOURCE_INTF_0		3
-#define DPU_VSYNC_SOURCE_INTF_1		4
-#define DPU_VSYNC_SOURCE_INTF_2		5
-#define DPU_VSYNC_SOURCE_INTF_3		6
-#define DPU_VSYNC_SOURCE_WD_TIMER_4	11
-#define DPU_VSYNC_SOURCE_WD_TIMER_3	12
-#define DPU_VSYNC_SOURCE_WD_TIMER_2	13
-#define DPU_VSYNC_SOURCE_WD_TIMER_1	14
-#define DPU_VSYNC_SOURCE_WD_TIMER_0	15
+enum dpu_vsync_source {
+	DPU_VSYNC_SOURCE_GPIO_0,
+	DPU_VSYNC_SOURCE_GPIO_1,
+	DPU_VSYNC_SOURCE_GPIO_2,
+	DPU_VSYNC_SOURCE_INTF_0 = 3,
+	DPU_VSYNC_SOURCE_INTF_1,
+	DPU_VSYNC_SOURCE_INTF_2,
+	DPU_VSYNC_SOURCE_INTF_3,
+	DPU_VSYNC_SOURCE_WD_TIMER_4 = 11,
+	DPU_VSYNC_SOURCE_WD_TIMER_3,
+	DPU_VSYNC_SOURCE_WD_TIMER_2,
+	DPU_VSYNC_SOURCE_WD_TIMER_1,
+	DPU_VSYNC_SOURCE_WD_TIMER_0,
+};
 
 enum dpu_hw_blk_type {
 	DPU_HW_BLK_TOP = 0,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
index 6f3dc98087df..5c9a7ede991e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
@@ -64,7 +64,7 @@  struct dpu_vsync_source_cfg {
 	u32 pp_count;
 	u32 frame_rate;
 	u32 ppnumber[PINGPONG_MAX];
-	u32 vsync_source;
+	enum dpu_vsync_source vsync_source;
 };
 
 /**