| Message ID | 20240520-dpu-handle-te-signal-v1-2-f273b42a089c@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/msm/dpu: handle non-default TE source pins | expand |
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>
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>
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 --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; }; /**
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(-)