| Message ID | 20240829-concurrent-wb-v1-10-502b16ae2ebb@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm/msm/dpu: Add Concurrent Writeback Support for DPU 10.x+ | expand |
On Thu, Aug 29, 2024 at 01:48:31PM GMT, Jessica Zhang wrote: > From: Esha Bharadwaj <quic_ebharadw@quicinc.com> > > Add function ops to allow hw_wb to configure CWB registers and adjust > the WB_MUX configuration to account for using dedicated CWB pingpong > blocks. > > Signed-off-by: Esha Bharadwaj <quic_ebharadw@quicinc.com> > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 69 ++++++++++++++++++++++++++++++- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 34 +++++++++++++++ > 2 files changed, 102 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c > index 93ff01c889b5..998c9fbd22a6 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c > @@ -51,6 +51,12 @@ > #define WB_CDP_CNTL 0x2B4 > #define WB_OUT_IMAGE_SIZE 0x2C0 > #define WB_OUT_XY 0x2C4 Empty line > +#define CWB_MUX 0x000 > +#define CWB_MODE 0x004 > + > +/* CWB mux block bit definitions */ > +#define CWB_MUX_MASK GENMASK(3, 0) > +#define CWB_MODE_MASK GENMASK(2, 0) > > static void dpu_hw_wb_setup_outaddress(struct dpu_hw_wb *ctx, > struct dpu_hw_wb_cfg *data) > @@ -173,7 +179,9 @@ static void dpu_hw_wb_bind_pingpong_blk( > mux_cfg = DPU_REG_READ(c, WB_MUX); > mux_cfg &= ~0xf; > > - if (pp) > + if (pp >= PINGPONG_CWB_0) > + mux_cfg |= (pp < PINGPONG_CWB_2) ? 0xd : 0xb; > + else if (pp) > mux_cfg |= (pp - PINGPONG_0) & 0x7; > else > mux_cfg |= 0xf; > @@ -237,3 +245,62 @@ struct dpu_hw_wb *dpu_hw_wb_init(struct drm_device *dev, > > return c; > } > + > +static void dpu_hw_cwb_setup_input_ctrl(struct dpu_hw_wb *ctx, > + enum dpu_cwb cwb_idx, > + const enum dpu_pingpong pp) Align to opening bracket, please. > +{ > + struct dpu_hw_blk_reg_map *c; > + int cwb_mux_cfg = 0xF; > + > + if (cwb_idx < CWB_0 || cwb_idx >= CWB_MAX) > + return; > + > + c = &ctx->cwb_hw[cwb_idx - CWB_0]; > + > + /* > + * The CWB_MUX register takes the pingpong index relative to > + * PINGPONG_CWB_0 and not PINGPONG_0 > + */ > + if ((pp != PINGPONG_NONE) && (pp < PINGPONG_MAX)) > + cwb_mux_cfg = FIELD_PREP(CWB_MUX_MASK, pp - PINGPONG_CWB_0); > + > + DPU_REG_WRITE(c, CWB_MUX, cwb_mux_cfg); > +} > + > +static void dpu_hw_cwb_setup_input_mode(struct dpu_hw_wb *ctx, > + enum dpu_cwb cwb_idx, > + const enum cwb_mode_input input) > +{ > + struct dpu_hw_blk_reg_map *c; > + int cwb_mux_cfg; > + > + if (cwb_idx < CWB_0 || cwb_idx >= CWB_MAX || input >= INPUT_MODE_MAX) > + return; > + > + c = &ctx->cwb_hw[cwb_idx - CWB_0]; > + cwb_mux_cfg = FIELD_PREP(CWB_MODE_MASK, input); > + > + DPU_REG_WRITE(c, CWB_MODE, cwb_mux_cfg); > +} > + > +static void _setup_cwb_ops(struct dpu_hw_wb_ops *ops) > +{ > + ops->setup_input_ctrl = dpu_hw_cwb_setup_input_ctrl; > + ops->setup_input_mode = dpu_hw_cwb_setup_input_mode; > +} > + > +struct dpu_hw_wb *dpu_hw_wb_init_with_cwb(struct drm_device *dev, > + const struct dpu_wb_cfg *cfg, > + void __iomem *addr, > + const struct dpu_mdss_version *mdss_rev, > + struct dpu_hw_blk_reg_map *cwb_hw) > +{ > + struct dpu_hw_wb *c = dpu_hw_wb_init(dev, cfg, addr, mdss_rev); > + > + c->cwb_hw = cwb_hw; > + > + _setup_cwb_ops(&c->ops); > + > + return c; > +} > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h > index 37497473e16c..1ff40f8325e5 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h > @@ -21,6 +21,12 @@ struct dpu_hw_wb_cfg { > struct drm_rect crop; > }; > > +enum cwb_mode_input { > + INPUT_MODE_LM_OUT, > + INPUT_MODE_DSPP_OUT, > + INPUT_MODE_MAX > +}; > + > /** > * > * struct dpu_hw_wb_ops : Interface to the wb hw driver functions > @@ -31,6 +37,8 @@ struct dpu_hw_wb_cfg { > * @setup_cdp: setup chroma down prefetch block for writeback block > * @setup_clk_force_ctrl: setup clock force control > * @bind_pingpong_blk: enable/disable the connection with ping-pong block > + * @setup_ctrl: setup ctrl register for cwb > + * @setup_mode: setup mode register for cwb > */ > struct dpu_hw_wb_ops { > void (*setup_outaddress)(struct dpu_hw_wb *ctx, > @@ -54,11 +62,20 @@ struct dpu_hw_wb_ops { > > void (*bind_pingpong_blk)(struct dpu_hw_wb *ctx, > const enum dpu_pingpong pp); > + > + void (*setup_input_ctrl)(struct dpu_hw_wb *ctx, > + enum dpu_cwb cwb_idx, > + const enum dpu_pingpong pp); > + > + void (*setup_input_mode)(struct dpu_hw_wb *ctx, > + enum dpu_cwb cwb_idx, > + const enum cwb_mode_input input); Judging by the usage, could you please add single setup_cwb() callback and let WB code call corresponding functions internally. > }; > > /** > * struct dpu_hw_wb : WB driver object > * @hw: block hardware details > + * @cwb_hw: block hardware details of cwb_muxes > * @idx: hardware index number within type > * @wb_hw_caps: hardware capabilities > * @ops: function pointers > @@ -66,6 +83,9 @@ struct dpu_hw_wb_ops { > struct dpu_hw_wb { > struct dpu_hw_blk_reg_map hw; > > + /* cwb data */ > + struct dpu_hw_blk_reg_map *cwb_hw; > + > /* wb path */ > int idx; > const struct dpu_wb_cfg *caps; > @@ -87,4 +107,18 @@ struct dpu_hw_wb *dpu_hw_wb_init(struct drm_device *dev, > void __iomem *addr, > const struct dpu_mdss_version *mdss_rev); > > +/** > + * dpu_hw_wb_init_with_cwb() - Initializes the writeback hw driver object with cwb. > + * @dev: Corresponding device for devres management > + * @cfg: wb_path catalog entry for which driver object is required > + * @addr: mapped register io address of MDP > + * @hw: array of cwb muxes > + * Return: Error code or allocated dpu_hw_wb context > + */ Please move the comment to the function body. This way make W=1 will check that it is valid. > +struct dpu_hw_wb *dpu_hw_wb_init_with_cwb(struct drm_device *dev, > + const struct dpu_wb_cfg *cfg, > + void __iomem *addr, > + const struct dpu_mdss_version *mdss_rev, > + struct dpu_hw_blk_reg_map *hw); > + > #endif /*_DPU_HW_WB_H */ > > -- > 2.34.1 >
On 8/30/2024 10:41 AM, Dmitry Baryshkov wrote: > On Thu, Aug 29, 2024 at 01:48:31PM GMT, Jessica Zhang wrote: >> From: Esha Bharadwaj <quic_ebharadw@quicinc.com> >> >> Add function ops to allow hw_wb to configure CWB registers and adjust >> the WB_MUX configuration to account for using dedicated CWB pingpong >> blocks. >> >> Signed-off-by: Esha Bharadwaj <quic_ebharadw@quicinc.com> >> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 69 ++++++++++++++++++++++++++++++- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 34 +++++++++++++++ >> 2 files changed, 102 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c >> index 93ff01c889b5..998c9fbd22a6 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c >> @@ -51,6 +51,12 @@ >> #define WB_CDP_CNTL 0x2B4 >> #define WB_OUT_IMAGE_SIZE 0x2C0 >> #define WB_OUT_XY 0x2C4 > > Empty line > >> +#define CWB_MUX 0x000 >> +#define CWB_MODE 0x004 >> + >> +/* CWB mux block bit definitions */ >> +#define CWB_MUX_MASK GENMASK(3, 0) >> +#define CWB_MODE_MASK GENMASK(2, 0) >> >> static void dpu_hw_wb_setup_outaddress(struct dpu_hw_wb *ctx, >> struct dpu_hw_wb_cfg *data) >> @@ -173,7 +179,9 @@ static void dpu_hw_wb_bind_pingpong_blk( >> mux_cfg = DPU_REG_READ(c, WB_MUX); >> mux_cfg &= ~0xf; >> >> - if (pp) >> + if (pp >= PINGPONG_CWB_0) >> + mux_cfg |= (pp < PINGPONG_CWB_2) ? 0xd : 0xb; >> + else if (pp) >> mux_cfg |= (pp - PINGPONG_0) & 0x7; >> else >> mux_cfg |= 0xf; >> @@ -237,3 +245,62 @@ struct dpu_hw_wb *dpu_hw_wb_init(struct drm_device *dev, >> >> return c; >> } >> + >> +static void dpu_hw_cwb_setup_input_ctrl(struct dpu_hw_wb *ctx, >> + enum dpu_cwb cwb_idx, >> + const enum dpu_pingpong pp) > > Align to opening bracket, please. Hi Dmitry, Acked for all comments here -- I'll squash the 2 cwb ops together. Thanks, Jessica Zhang > >> +{ >> + struct dpu_hw_blk_reg_map *c; >> + int cwb_mux_cfg = 0xF; >> + >> + if (cwb_idx < CWB_0 || cwb_idx >= CWB_MAX) >> + return; >> + >> + c = &ctx->cwb_hw[cwb_idx - CWB_0]; >> + >> + /* >> + * The CWB_MUX register takes the pingpong index relative to >> + * PINGPONG_CWB_0 and not PINGPONG_0 >> + */ >> + if ((pp != PINGPONG_NONE) && (pp < PINGPONG_MAX)) >> + cwb_mux_cfg = FIELD_PREP(CWB_MUX_MASK, pp - PINGPONG_CWB_0); >> + >> + DPU_REG_WRITE(c, CWB_MUX, cwb_mux_cfg); >> +} >> + >> +static void dpu_hw_cwb_setup_input_mode(struct dpu_hw_wb *ctx, >> + enum dpu_cwb cwb_idx, >> + const enum cwb_mode_input input) >> +{ >> + struct dpu_hw_blk_reg_map *c; >> + int cwb_mux_cfg; >> + >> + if (cwb_idx < CWB_0 || cwb_idx >= CWB_MAX || input >= INPUT_MODE_MAX) >> + return; >> + >> + c = &ctx->cwb_hw[cwb_idx - CWB_0]; >> + cwb_mux_cfg = FIELD_PREP(CWB_MODE_MASK, input); >> + >> + DPU_REG_WRITE(c, CWB_MODE, cwb_mux_cfg); >> +} >> + >> +static void _setup_cwb_ops(struct dpu_hw_wb_ops *ops) >> +{ >> + ops->setup_input_ctrl = dpu_hw_cwb_setup_input_ctrl; >> + ops->setup_input_mode = dpu_hw_cwb_setup_input_mode; >> +} >> + >> +struct dpu_hw_wb *dpu_hw_wb_init_with_cwb(struct drm_device *dev, >> + const struct dpu_wb_cfg *cfg, >> + void __iomem *addr, >> + const struct dpu_mdss_version *mdss_rev, >> + struct dpu_hw_blk_reg_map *cwb_hw) >> +{ >> + struct dpu_hw_wb *c = dpu_hw_wb_init(dev, cfg, addr, mdss_rev); >> + >> + c->cwb_hw = cwb_hw; >> + >> + _setup_cwb_ops(&c->ops); >> + >> + return c; >> +} >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h >> index 37497473e16c..1ff40f8325e5 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h >> @@ -21,6 +21,12 @@ struct dpu_hw_wb_cfg { >> struct drm_rect crop; >> }; >> >> +enum cwb_mode_input { >> + INPUT_MODE_LM_OUT, >> + INPUT_MODE_DSPP_OUT, >> + INPUT_MODE_MAX >> +}; >> + >> /** >> * >> * struct dpu_hw_wb_ops : Interface to the wb hw driver functions >> @@ -31,6 +37,8 @@ struct dpu_hw_wb_cfg { >> * @setup_cdp: setup chroma down prefetch block for writeback block >> * @setup_clk_force_ctrl: setup clock force control >> * @bind_pingpong_blk: enable/disable the connection with ping-pong block >> + * @setup_ctrl: setup ctrl register for cwb >> + * @setup_mode: setup mode register for cwb >> */ >> struct dpu_hw_wb_ops { >> void (*setup_outaddress)(struct dpu_hw_wb *ctx, >> @@ -54,11 +62,20 @@ struct dpu_hw_wb_ops { >> >> void (*bind_pingpong_blk)(struct dpu_hw_wb *ctx, >> const enum dpu_pingpong pp); >> + >> + void (*setup_input_ctrl)(struct dpu_hw_wb *ctx, >> + enum dpu_cwb cwb_idx, >> + const enum dpu_pingpong pp); >> + >> + void (*setup_input_mode)(struct dpu_hw_wb *ctx, >> + enum dpu_cwb cwb_idx, >> + const enum cwb_mode_input input); > > Judging by the usage, could you please add single setup_cwb() callback > and let WB code call corresponding functions internally. > >> }; >> >> /** >> * struct dpu_hw_wb : WB driver object >> * @hw: block hardware details >> + * @cwb_hw: block hardware details of cwb_muxes >> * @idx: hardware index number within type >> * @wb_hw_caps: hardware capabilities >> * @ops: function pointers >> @@ -66,6 +83,9 @@ struct dpu_hw_wb_ops { >> struct dpu_hw_wb { >> struct dpu_hw_blk_reg_map hw; >> >> + /* cwb data */ >> + struct dpu_hw_blk_reg_map *cwb_hw; >> + >> /* wb path */ >> int idx; >> const struct dpu_wb_cfg *caps; >> @@ -87,4 +107,18 @@ struct dpu_hw_wb *dpu_hw_wb_init(struct drm_device *dev, >> void __iomem *addr, >> const struct dpu_mdss_version *mdss_rev); >> >> +/** >> + * dpu_hw_wb_init_with_cwb() - Initializes the writeback hw driver object with cwb. >> + * @dev: Corresponding device for devres management >> + * @cfg: wb_path catalog entry for which driver object is required >> + * @addr: mapped register io address of MDP >> + * @hw: array of cwb muxes >> + * Return: Error code or allocated dpu_hw_wb context >> + */ > > Please move the comment to the function body. This way make W=1 will > check that it is valid. > >> +struct dpu_hw_wb *dpu_hw_wb_init_with_cwb(struct drm_device *dev, >> + const struct dpu_wb_cfg *cfg, >> + void __iomem *addr, >> + const struct dpu_mdss_version *mdss_rev, >> + struct dpu_hw_blk_reg_map *hw); >> + >> #endif /*_DPU_HW_WB_H */ >> >> -- >> 2.34.1 >> > > -- > With best wishes > Dmitry
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c index 93ff01c889b5..998c9fbd22a6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c @@ -51,6 +51,12 @@ #define WB_CDP_CNTL 0x2B4 #define WB_OUT_IMAGE_SIZE 0x2C0 #define WB_OUT_XY 0x2C4 +#define CWB_MUX 0x000 +#define CWB_MODE 0x004 + +/* CWB mux block bit definitions */ +#define CWB_MUX_MASK GENMASK(3, 0) +#define CWB_MODE_MASK GENMASK(2, 0) static void dpu_hw_wb_setup_outaddress(struct dpu_hw_wb *ctx, struct dpu_hw_wb_cfg *data) @@ -173,7 +179,9 @@ static void dpu_hw_wb_bind_pingpong_blk( mux_cfg = DPU_REG_READ(c, WB_MUX); mux_cfg &= ~0xf; - if (pp) + if (pp >= PINGPONG_CWB_0) + mux_cfg |= (pp < PINGPONG_CWB_2) ? 0xd : 0xb; + else if (pp) mux_cfg |= (pp - PINGPONG_0) & 0x7; else mux_cfg |= 0xf; @@ -237,3 +245,62 @@ struct dpu_hw_wb *dpu_hw_wb_init(struct drm_device *dev, return c; } + +static void dpu_hw_cwb_setup_input_ctrl(struct dpu_hw_wb *ctx, + enum dpu_cwb cwb_idx, + const enum dpu_pingpong pp) +{ + struct dpu_hw_blk_reg_map *c; + int cwb_mux_cfg = 0xF; + + if (cwb_idx < CWB_0 || cwb_idx >= CWB_MAX) + return; + + c = &ctx->cwb_hw[cwb_idx - CWB_0]; + + /* + * The CWB_MUX register takes the pingpong index relative to + * PINGPONG_CWB_0 and not PINGPONG_0 + */ + if ((pp != PINGPONG_NONE) && (pp < PINGPONG_MAX)) + cwb_mux_cfg = FIELD_PREP(CWB_MUX_MASK, pp - PINGPONG_CWB_0); + + DPU_REG_WRITE(c, CWB_MUX, cwb_mux_cfg); +} + +static void dpu_hw_cwb_setup_input_mode(struct dpu_hw_wb *ctx, + enum dpu_cwb cwb_idx, + const enum cwb_mode_input input) +{ + struct dpu_hw_blk_reg_map *c; + int cwb_mux_cfg; + + if (cwb_idx < CWB_0 || cwb_idx >= CWB_MAX || input >= INPUT_MODE_MAX) + return; + + c = &ctx->cwb_hw[cwb_idx - CWB_0]; + cwb_mux_cfg = FIELD_PREP(CWB_MODE_MASK, input); + + DPU_REG_WRITE(c, CWB_MODE, cwb_mux_cfg); +} + +static void _setup_cwb_ops(struct dpu_hw_wb_ops *ops) +{ + ops->setup_input_ctrl = dpu_hw_cwb_setup_input_ctrl; + ops->setup_input_mode = dpu_hw_cwb_setup_input_mode; +} + +struct dpu_hw_wb *dpu_hw_wb_init_with_cwb(struct drm_device *dev, + const struct dpu_wb_cfg *cfg, + void __iomem *addr, + const struct dpu_mdss_version *mdss_rev, + struct dpu_hw_blk_reg_map *cwb_hw) +{ + struct dpu_hw_wb *c = dpu_hw_wb_init(dev, cfg, addr, mdss_rev); + + c->cwb_hw = cwb_hw; + + _setup_cwb_ops(&c->ops); + + return c; +} diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h index 37497473e16c..1ff40f8325e5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h @@ -21,6 +21,12 @@ struct dpu_hw_wb_cfg { struct drm_rect crop; }; +enum cwb_mode_input { + INPUT_MODE_LM_OUT, + INPUT_MODE_DSPP_OUT, + INPUT_MODE_MAX +}; + /** * * struct dpu_hw_wb_ops : Interface to the wb hw driver functions @@ -31,6 +37,8 @@ struct dpu_hw_wb_cfg { * @setup_cdp: setup chroma down prefetch block for writeback block * @setup_clk_force_ctrl: setup clock force control * @bind_pingpong_blk: enable/disable the connection with ping-pong block + * @setup_ctrl: setup ctrl register for cwb + * @setup_mode: setup mode register for cwb */ struct dpu_hw_wb_ops { void (*setup_outaddress)(struct dpu_hw_wb *ctx, @@ -54,11 +62,20 @@ struct dpu_hw_wb_ops { void (*bind_pingpong_blk)(struct dpu_hw_wb *ctx, const enum dpu_pingpong pp); + + void (*setup_input_ctrl)(struct dpu_hw_wb *ctx, + enum dpu_cwb cwb_idx, + const enum dpu_pingpong pp); + + void (*setup_input_mode)(struct dpu_hw_wb *ctx, + enum dpu_cwb cwb_idx, + const enum cwb_mode_input input); }; /** * struct dpu_hw_wb : WB driver object * @hw: block hardware details + * @cwb_hw: block hardware details of cwb_muxes * @idx: hardware index number within type * @wb_hw_caps: hardware capabilities * @ops: function pointers @@ -66,6 +83,9 @@ struct dpu_hw_wb_ops { struct dpu_hw_wb { struct dpu_hw_blk_reg_map hw; + /* cwb data */ + struct dpu_hw_blk_reg_map *cwb_hw; + /* wb path */ int idx; const struct dpu_wb_cfg *caps; @@ -87,4 +107,18 @@ struct dpu_hw_wb *dpu_hw_wb_init(struct drm_device *dev, void __iomem *addr, const struct dpu_mdss_version *mdss_rev); +/** + * dpu_hw_wb_init_with_cwb() - Initializes the writeback hw driver object with cwb. + * @dev: Corresponding device for devres management + * @cfg: wb_path catalog entry for which driver object is required + * @addr: mapped register io address of MDP + * @hw: array of cwb muxes + * Return: Error code or allocated dpu_hw_wb context + */ +struct dpu_hw_wb *dpu_hw_wb_init_with_cwb(struct drm_device *dev, + const struct dpu_wb_cfg *cfg, + void __iomem *addr, + const struct dpu_mdss_version *mdss_rev, + struct dpu_hw_blk_reg_map *hw); + #endif /*_DPU_HW_WB_H */