From: Neil Armstrong <neil.armstrong@linaro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Rob Clark <robdclark@gmail.com>,
Abhinav Kumar <quic_abhinavk@quicinc.com>,
Sean Paul <sean@poorly.run>,
Marijn Suijten <marijn.suijten@somainline.org>,
David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/5] drm/msm/dpu: add setup_clk_force_ctrl() op to sspp & wb
Date: Wed, 11 Oct 2023 15:15:32 +0200 [thread overview]
Message-ID: <cfc98b1d-37fa-435e-bf54-0ce01c754adf@linaro.org> (raw)
In-Reply-To: <CAA8EJpps7g6LWFUS87j2oGwMW6He=MDSm_GaR_zOyQPHUvGReg@mail.gmail.com>
On 11/10/2023 14:45, Dmitry Baryshkov wrote:
> On Wed, 11 Oct 2023 at 14:59, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>>
>> Starting from SM8550, the SSPP & WB clock controls are moved
>> the SSPP and WB register range, as it's called "VBIF_CLK_SPLIT"
>> downstream.
>>
>> Implement setup_clk_force_ctrl() only starting from major version 9
>> which corresponds to SM8550 MDSS.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>
> With two minor issues below fixed:
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 21 ++++++++++++++++++---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 12 +++++++++++-
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 20 +++++++++++++++++---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 7 ++++++-
>> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 4 ++--
>> 5 files changed, 54 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
>> index f2192de93713..5fd213ed6491 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
>> @@ -69,6 +69,7 @@
>> #define SSPP_EXCL_REC_XY_REC1 0x188
>> #define SSPP_EXCL_REC_SIZE 0x1B4
>> #define SSPP_EXCL_REC_XY 0x1B8
>> +#define SSPP_CLK_CTRL 0x330
>>
>> /* SSPP_SRC_OP_MODE & OP_MODE_REC1 */
>> #define MDSS_MDP_OP_DEINTERLACE BIT(22)
>> @@ -581,8 +582,18 @@ static void dpu_hw_sspp_setup_cdp(struct dpu_sw_pipe *pipe,
>> dpu_setup_cdp(&ctx->hw, cdp_cntl_offset, fmt, enable);
>> }
>>
>> +static bool dpu_hw_sspp_setup_clk_force_ctrl(struct dpu_hw_sspp *ctx, bool enable)
>> +{
>> + struct dpu_clk_ctrl_reg sspp_clk_ctrl = {
>
> Nit: static const?
Yep will add
>
>> + .reg_off = SSPP_CLK_CTRL,
>> + .bit_off = 0
>> + };
>> +
>> + return dpu_hw_clk_force_ctrl(&ctx->hw, &sspp_clk_ctrl, enable);
>> +}
>> +
>> static void _setup_layer_ops(struct dpu_hw_sspp *c,
>> - unsigned long features)
>> + unsigned long features, const struct dpu_mdss_version *mdss_rev)
>> {
>> c->ops.setup_format = dpu_hw_sspp_setup_format;
>> c->ops.setup_rects = dpu_hw_sspp_setup_rects;
>> @@ -612,6 +623,9 @@ static void _setup_layer_ops(struct dpu_hw_sspp *c,
>>
>> if (test_bit(DPU_SSPP_CDP, &features))
>> c->ops.setup_cdp = dpu_hw_sspp_setup_cdp;
>> +
>> + if (mdss_rev->core_major_ver >= 9)
>> + c->ops.setup_clk_force_ctrl = dpu_hw_sspp_setup_clk_force_ctrl;
>> }
>>
>> #ifdef CONFIG_DEBUG_FS
>> @@ -672,7 +686,8 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
>> #endif
>>
>> struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
>> - void __iomem *addr, const struct msm_mdss_data *mdss_data)
>> + void __iomem *addr, const struct msm_mdss_data *mdss_data,
>> + const struct dpu_mdss_version *mdss_rev)
>> {
>> struct dpu_hw_sspp *hw_pipe;
>>
>> @@ -690,7 +705,7 @@ struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
>> hw_pipe->ubwc = mdss_data;
>> hw_pipe->idx = cfg->id;
>> hw_pipe->cap = cfg;
>> - _setup_layer_ops(hw_pipe, hw_pipe->cap->features);
>> + _setup_layer_ops(hw_pipe, hw_pipe->cap->features, mdss_rev);
>>
>> return hw_pipe;
>> }
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
>> index cbf4f95ff0fd..f93969fddb22 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
>> @@ -271,6 +271,14 @@ struct dpu_hw_sspp_ops {
>> void (*setup_qos_ctrl)(struct dpu_hw_sspp *ctx,
>> bool danger_safe_en);
>>
>> + /**
>> + * setup_clk_force_ctrl - setup clock force control
>> + * @ctx: Pointer to pipe context
>> + * @enable: enable clock force if true
>> + */
>> + bool (*setup_clk_force_ctrl)(struct dpu_hw_sspp *ctx,
>> + bool enable);
>> +
>> /**
>> * setup_histogram - setup histograms
>> * @ctx: Pointer to pipe context
>> @@ -334,9 +342,11 @@ struct dpu_kms;
>> * @cfg: Pipe catalog entry for which driver object is required
>> * @addr: Mapped register io address of MDP
>> * @mdss_data: UBWC / MDSS configuration data
>> + * @mdss_rev: dpu core's major and minor versions
>> */
>> struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
>> - void __iomem *addr, const struct msm_mdss_data *mdss_data);
>> + void __iomem *addr, const struct msm_mdss_data *mdss_data,
>> + const struct dpu_mdss_version *mdss_rev);
>>
>> /**
>> * dpu_hw_sspp_destroy(): Destroys SSPP driver context
>> 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 ebc416400382..374c2c64c9e4 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
>> @@ -43,6 +43,7 @@
>> #define WB_MUX 0x150
>> #define WB_CROP_CTRL 0x154
>> #define WB_CROP_OFFSET 0x158
>> +#define WB_CLK_CTRL 0x178
>> #define WB_CSC_BASE 0x260
>> #define WB_DST_ADDR_SW_STATUS 0x2B0
>> #define WB_CDP_CNTL 0x2B4
>> @@ -175,8 +176,18 @@ static void dpu_hw_wb_bind_pingpong_blk(
>> DPU_REG_WRITE(c, WB_MUX, mux_cfg);
>> }
>>
>> +static bool dpu_hw_wb_setup_clk_force_ctrl(struct dpu_hw_wb *ctx, bool enable)
>> +{
>> + struct dpu_clk_ctrl_reg wb_clk_ctrl = {
>
> And here too, static const. We can even move them away from the function.
Since it's only used on the function, let's let it here in static const
Neil
>
>> + .reg_off = WB_CLK_CTRL,
>> + .bit_off = 0
>> + };
>> +
>> + return dpu_hw_clk_force_ctrl(&ctx->hw, &wb_clk_ctrl, enable);
>> +}
>> +
>> static void _setup_wb_ops(struct dpu_hw_wb_ops *ops,
>> - unsigned long features)
>> + unsigned long features, const struct dpu_mdss_version *mdss_rev)
>> {
>> ops->setup_outaddress = dpu_hw_wb_setup_outaddress;
>> ops->setup_outformat = dpu_hw_wb_setup_format;
>> @@ -192,10 +203,13 @@ static void _setup_wb_ops(struct dpu_hw_wb_ops *ops,
>>
>> if (test_bit(DPU_WB_INPUT_CTRL, &features))
>> ops->bind_pingpong_blk = dpu_hw_wb_bind_pingpong_blk;
>> +
>> + if (mdss_rev->core_major_ver >= 9)
>> + ops->setup_clk_force_ctrl = dpu_hw_wb_setup_clk_force_ctrl;
>> }
>>
>> struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
>> - void __iomem *addr)
>> + void __iomem *addr, const struct dpu_mdss_version *mdss_rev)
>> {
>> struct dpu_hw_wb *c;
>>
>> @@ -212,7 +226,7 @@ struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
>> /* Assign ops */
>> c->idx = cfg->id;
>> c->caps = cfg;
>> - _setup_wb_ops(&c->ops, c->caps->features);
>> + _setup_wb_ops(&c->ops, c->caps->features, mdss_rev);
>>
>> 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 2d7db2efa3d0..88792f450a92 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
>> @@ -29,6 +29,7 @@ struct dpu_hw_wb_cfg {
>> * @setup_outformat: setup output format of writeback block from writeback job
>> * @setup_qos_lut: setup qos LUT for writeback block based on input
>> * @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
>> */
>> struct dpu_hw_wb_ops {
>> @@ -48,6 +49,9 @@ struct dpu_hw_wb_ops {
>> const struct dpu_format *fmt,
>> bool enable);
>>
>> + bool (*setup_clk_force_ctrl)(struct dpu_hw_wb *ctx,
>> + bool enable);
>> +
>> void (*bind_pingpong_blk)(struct dpu_hw_wb *ctx,
>> const enum dpu_pingpong pp);
>> };
>> @@ -74,10 +78,11 @@ struct dpu_hw_wb {
>> * dpu_hw_wb_init() - Initializes the writeback hw driver object.
>> * @cfg: wb_path catalog entry for which driver object is required
>> * @addr: mapped register io address of MDP
>> + * @mdss_rev: dpu core's major and minor versions
>> * Return: Error code or allocated dpu_hw_wb context
>> */
>> struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
>> - void __iomem *addr);
>> + void __iomem *addr, const struct dpu_mdss_version *mdss_rev);
>>
>> /**
>> * dpu_hw_wb_destroy(): Destroy writeback hw driver object.
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>> index f9215643c71a..f363bcfdfd70 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>> @@ -175,7 +175,7 @@ int dpu_rm_init(struct dpu_rm *rm,
>> struct dpu_hw_wb *hw;
>> const struct dpu_wb_cfg *wb = &cat->wb[i];
>>
>> - hw = dpu_hw_wb_init(wb, mmio);
>> + hw = dpu_hw_wb_init(wb, mmio, cat->mdss_ver);
>> if (IS_ERR(hw)) {
>> rc = PTR_ERR(hw);
>> DPU_ERROR("failed wb object creation: err %d\n", rc);
>> @@ -231,7 +231,7 @@ int dpu_rm_init(struct dpu_rm *rm,
>> struct dpu_hw_sspp *hw;
>> const struct dpu_sspp_cfg *sspp = &cat->sspp[i];
>>
>> - hw = dpu_hw_sspp_init(sspp, mmio, mdss_data);
>> + hw = dpu_hw_sspp_init(sspp, mmio, mdss_data, cat->mdss_ver);
>> if (IS_ERR(hw)) {
>> rc = PTR_ERR(hw);
>> DPU_ERROR("failed sspp object creation: err %d\n", rc);
>>
>> --
>> 2.34.1
>>
>
>
next prev parent reply other threads:[~2023-10-11 13:15 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-11 11:59 [PATCH v2 0/5] drm/msm/dpu: correctly implement SSPP & WB Clock Control Split Neil Armstrong
2023-10-11 11:59 ` [PATCH v2 1/5] drm/msm/dpu: create a dpu_hw_clk_force_ctrl() helper Neil Armstrong
2023-10-11 11:59 ` [PATCH v2 2/5] drm/msm/dpu: add setup_clk_force_ctrl() op to sspp & wb Neil Armstrong
2023-10-11 12:45 ` Dmitry Baryshkov
2023-10-11 13:15 ` Neil Armstrong [this message]
2023-10-11 11:59 ` [PATCH v2 3/5] drm/msm/dpu: move setup_force_clk_ctrl handling into plane and wb Neil Armstrong
2023-10-11 12:55 ` Dmitry Baryshkov
2023-10-11 11:59 ` [PATCH v2 4/5] drm/msm/dpu: sm8550: remove unused VIG and DMA clock controls entries Neil Armstrong
2023-10-11 12:55 ` Dmitry Baryshkov
2023-10-11 11:59 ` [PATCH v2 5/5] drm/msm/dpu: enable writeback on SM8550 Neil Armstrong
2023-10-11 12:46 ` Dmitry Baryshkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cfc98b1d-37fa-435e-bf54-0ce01c754adf@linaro.org \
--to=neil.armstrong@linaro.org \
--cc=airlied@gmail.com \
--cc=daniel@ffwll.ch \
--cc=dmitry.baryshkov@linaro.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marijn.suijten@somainline.org \
--cc=quic_abhinavk@quicinc.com \
--cc=robdclark@gmail.com \
--cc=sean@poorly.run \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox