Re: [Freedreno] [PATCH v3 22/27] drm/msm/dpu: rework dpu_plane_sspp_atomic_update()
On 07/02/2023 02:22, Abhinav Kumar wrote:
On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
Split pipe-dependent code from dpu_plane_sspp_atomic_update() into the
sspp-dependent?
separate function dpu_plane_sspp_update_pipe(). This is one of
preparational steps to add r_pipe support.
Signed-off-by: Dmitry Baryshkov
Just a couple of minor comments below but otherwise this split up lgtm
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 113 --
1 file changed, 63 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 0986e740b978..f94e132733f3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -404,12 +404,13 @@ static void _dpu_plane_set_qos_ctrl(struct
drm_plane *plane,
* _dpu_plane_set_ot_limit - set OT limit for the given plane
* @plane: Pointer to drm plane
* @pipe: Pointer to software pipe
- * @crtc: Pointer to drm crtc
* @pipe_cfg: Pointer to pipe configuration
+ * @frame_rate: CRTC's frame rate
Can you please check the spacing here. There seems to be an extra tab
before the CRTC's frame rate
I checked, the ident is correct here. It uses tabs, maybe that confuses
your mailer.
*/
static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
struct dpu_sw_pipe *pipe,
- struct drm_crtc *crtc, struct dpu_hw_sspp_cfg *pipe_cfg)
+ struct dpu_hw_sspp_cfg *pipe_cfg,
+ int frame_rate)
{
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_vbif_set_ot_params ot_params;
@@ -421,7 +422,7 @@ static void _dpu_plane_set_ot_limit(struct
drm_plane *plane,
ot_params.width = drm_rect_width(_cfg->src_rect);
ot_params.height = drm_rect_height(_cfg->src_rect);
ot_params.is_wfd = !pdpu->is_rt_pipe;
- ot_params.frame_rate = drm_mode_vrefresh(>mode);
+ ot_params.frame_rate = frame_rate;
ot_params.vbif_idx = VBIF_RT;
ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
ot_params.rd = true;
@@ -457,26 +458,6 @@ static void _dpu_plane_set_qos_remap(struct
drm_plane *plane,
dpu_vbif_set_qos_remap(dpu_kms, _params);
}
-static void _dpu_plane_set_scanout(struct drm_plane *plane,
- struct dpu_plane_state *pstate,
- struct drm_framebuffer *fb)
-{
- struct dpu_plane *pdpu = to_dpu_plane(plane);
- struct dpu_kms *kms = _dpu_plane_get_kms(>base);
- struct msm_gem_address_space *aspace = kms->base.aspace;
- struct dpu_hw_fmt_layout layout;
- int ret;
-
- ret = dpu_format_populate_layout(aspace, fb, );
- if (ret)
- DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
- else if (pstate->pipe.sspp->ops.setup_sourceaddress) {
- trace_dpu_plane_set_scanout(>pipe,
- );
- pstate->pipe.sspp->ops.setup_sourceaddress(>pipe,
);
- }
-}
-
static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw,
uint32_t src_w, uint32_t src_h, uint32_t dst_w, uint32_t dst_h,
struct dpu_hw_scaler3_cfg *scale_cfg,
@@ -1102,35 +1083,25 @@ void dpu_plane_set_error(struct drm_plane
*plane, bool error)
pdpu->is_error = error;
}
-static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
+static void dpu_plane_sspp_update_pipe(struct drm_plane *plane,
+ struct dpu_sw_pipe *pipe,
+ struct dpu_hw_sspp_cfg *pipe_cfg,
You can call this parameter sspp_cfg instead of pipe_cfg?
+ const struct dpu_format *fmt,
+ int frame_rate,
+ struct dpu_hw_fmt_layout *layout)
{
uint32_t src_flags;
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct drm_plane_state *state = plane->state;
struct dpu_plane_state *pstate = to_dpu_plane_state(state);
- struct dpu_sw_pipe *pipe = >pipe;
- struct drm_crtc *crtc = state->crtc;
- struct drm_framebuffer *fb = state->fb;
- bool is_rt_pipe;
- const struct dpu_format *fmt =
- to_dpu_format(msm_framebuffer_format(fb));
- struct dpu_hw_sspp_cfg *pipe_cfg = >pipe_cfg;
- _dpu_plane_set_scanout(plane, pstate, fb);
-
- pstate->pending = true;
-
- is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
- pstate->needs_qos_remap |= (is_rt_pipe != pdpu->is_rt_pipe);
- pdpu->is_rt_pipe = is_rt_pipe;
+ if (layout && pipe->sspp->ops.setup_sourceaddress) {
+ trace_dpu_plane_set_scanout(pipe, layout);
+ pipe->sspp->ops.setup_sourceaddress(pipe, layout);
+ }
_dpu_plane_set_qos_ctrl(plane, pipe, false,
DPU_PLANE_QOS_PANIC_CTRL);
- DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u "
DRM_RECT_FMT
- ", %4.4s ubwc %d\n", fb->base.id,
DRM_RECT_FP_ARG(>src),
- crtc->base.id, DRM_RECT_ARG(>dst),
- (char *)>base.pixel_format, DPU_FORMAT_IS_UBWC(fmt));
-
Re: [Freedreno] [PATCH v3 22/27] drm/msm/dpu: rework dpu_plane_sspp_atomic_update()
On 07/02/2023 02:22, Abhinav Kumar wrote: On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote: Split pipe-dependent code from dpu_plane_sspp_atomic_update() into the sspp-dependent? No, this is really pipe-dependent. It takes dpu_sw_pipe and dpu_sw_pipe_cfg arguments. separate function dpu_plane_sspp_update_pipe(). This is one of preparational steps to add r_pipe support. Signed-off-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [Freedreno] [PATCH v3 22/27] drm/msm/dpu: rework dpu_plane_sspp_atomic_update()
On 07/02/2023 02:22, Abhinav Kumar wrote:
On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
Split pipe-dependent code from dpu_plane_sspp_atomic_update() into the
sspp-dependent?
separate function dpu_plane_sspp_update_pipe(). This is one of
preparational steps to add r_pipe support.
Signed-off-by: Dmitry Baryshkov
Just a couple of minor comments below but otherwise this split up lgtm
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 113 --
1 file changed, 63 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 0986e740b978..f94e132733f3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -404,12 +404,13 @@ static void _dpu_plane_set_qos_ctrl(struct
drm_plane *plane,
* _dpu_plane_set_ot_limit - set OT limit for the given plane
* @plane: Pointer to drm plane
* @pipe: Pointer to software pipe
- * @crtc: Pointer to drm crtc
* @pipe_cfg: Pointer to pipe configuration
+ * @frame_rate: CRTC's frame rate
Can you please check the spacing here. There seems to be an extra tab
before the CRTC's frame rate
*/
static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
struct dpu_sw_pipe *pipe,
- struct drm_crtc *crtc, struct dpu_hw_sspp_cfg *pipe_cfg)
+ struct dpu_hw_sspp_cfg *pipe_cfg,
+ int frame_rate)
{
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_vbif_set_ot_params ot_params;
@@ -421,7 +422,7 @@ static void _dpu_plane_set_ot_limit(struct
drm_plane *plane,
ot_params.width = drm_rect_width(_cfg->src_rect);
ot_params.height = drm_rect_height(_cfg->src_rect);
ot_params.is_wfd = !pdpu->is_rt_pipe;
- ot_params.frame_rate = drm_mode_vrefresh(>mode);
+ ot_params.frame_rate = frame_rate;
ot_params.vbif_idx = VBIF_RT;
ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
ot_params.rd = true;
@@ -457,26 +458,6 @@ static void _dpu_plane_set_qos_remap(struct
drm_plane *plane,
dpu_vbif_set_qos_remap(dpu_kms, _params);
}
-static void _dpu_plane_set_scanout(struct drm_plane *plane,
- struct dpu_plane_state *pstate,
- struct drm_framebuffer *fb)
-{
- struct dpu_plane *pdpu = to_dpu_plane(plane);
- struct dpu_kms *kms = _dpu_plane_get_kms(>base);
- struct msm_gem_address_space *aspace = kms->base.aspace;
- struct dpu_hw_fmt_layout layout;
- int ret;
-
- ret = dpu_format_populate_layout(aspace, fb, );
- if (ret)
- DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
- else if (pstate->pipe.sspp->ops.setup_sourceaddress) {
- trace_dpu_plane_set_scanout(>pipe,
- );
- pstate->pipe.sspp->ops.setup_sourceaddress(>pipe,
);
- }
-}
-
static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw,
uint32_t src_w, uint32_t src_h, uint32_t dst_w, uint32_t dst_h,
struct dpu_hw_scaler3_cfg *scale_cfg,
@@ -1102,35 +1083,25 @@ void dpu_plane_set_error(struct drm_plane
*plane, bool error)
pdpu->is_error = error;
}
-static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
+static void dpu_plane_sspp_update_pipe(struct drm_plane *plane,
+ struct dpu_sw_pipe *pipe,
+ struct dpu_hw_sspp_cfg *pipe_cfg,
You can call this parameter sspp_cfg instead of pipe_cfg?
I think, I'll add a commit renaming dpu_hw_sspp_cfg to dpu_sw_pipe_cfg,
it would be simmetrical to dpu_sw_pipe then.
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH v3 22/27] drm/msm/dpu: rework dpu_plane_sspp_atomic_update()
On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
Split pipe-dependent code from dpu_plane_sspp_atomic_update() into the
sspp-dependent?
separate function dpu_plane_sspp_update_pipe(). This is one of
preparational steps to add r_pipe support.
Signed-off-by: Dmitry Baryshkov
Just a couple of minor comments below but otherwise this split up lgtm
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 113 --
1 file changed, 63 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 0986e740b978..f94e132733f3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -404,12 +404,13 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane
*plane,
* _dpu_plane_set_ot_limit - set OT limit for the given plane
* @plane:Pointer to drm plane
* @pipe: Pointer to software pipe
- * @crtc: Pointer to drm crtc
* @pipe_cfg: Pointer to pipe configuration
+ * @frame_rate:CRTC's frame rate
Can you please check the spacing here. There seems to be an extra tab
before the CRTC's frame rate
*/
static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
struct dpu_sw_pipe *pipe,
- struct drm_crtc *crtc, struct dpu_hw_sspp_cfg *pipe_cfg)
+ struct dpu_hw_sspp_cfg *pipe_cfg,
+ int frame_rate)
{
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_vbif_set_ot_params ot_params;
@@ -421,7 +422,7 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
ot_params.width = drm_rect_width(_cfg->src_rect);
ot_params.height = drm_rect_height(_cfg->src_rect);
ot_params.is_wfd = !pdpu->is_rt_pipe;
- ot_params.frame_rate = drm_mode_vrefresh(>mode);
+ ot_params.frame_rate = frame_rate;
ot_params.vbif_idx = VBIF_RT;
ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
ot_params.rd = true;
@@ -457,26 +458,6 @@ static void _dpu_plane_set_qos_remap(struct drm_plane
*plane,
dpu_vbif_set_qos_remap(dpu_kms, _params);
}
-static void _dpu_plane_set_scanout(struct drm_plane *plane,
- struct dpu_plane_state *pstate,
- struct drm_framebuffer *fb)
-{
- struct dpu_plane *pdpu = to_dpu_plane(plane);
- struct dpu_kms *kms = _dpu_plane_get_kms(>base);
- struct msm_gem_address_space *aspace = kms->base.aspace;
- struct dpu_hw_fmt_layout layout;
- int ret;
-
- ret = dpu_format_populate_layout(aspace, fb, );
- if (ret)
- DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
- else if (pstate->pipe.sspp->ops.setup_sourceaddress) {
- trace_dpu_plane_set_scanout(>pipe,
- );
- pstate->pipe.sspp->ops.setup_sourceaddress(>pipe,
);
- }
-}
-
static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw,
uint32_t src_w, uint32_t src_h, uint32_t dst_w, uint32_t dst_h,
struct dpu_hw_scaler3_cfg *scale_cfg,
@@ -1102,35 +1083,25 @@ void dpu_plane_set_error(struct drm_plane *plane, bool
error)
pdpu->is_error = error;
}
-static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
+static void dpu_plane_sspp_update_pipe(struct drm_plane *plane,
+ struct dpu_sw_pipe *pipe,
+ struct dpu_hw_sspp_cfg *pipe_cfg,
You can call this parameter sspp_cfg instead of pipe_cfg?
+ const struct dpu_format *fmt,
+ int frame_rate,
+ struct dpu_hw_fmt_layout *layout)
{
uint32_t src_flags;
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct drm_plane_state *state = plane->state;
struct dpu_plane_state *pstate = to_dpu_plane_state(state);
- struct dpu_sw_pipe *pipe = >pipe;
- struct drm_crtc *crtc = state->crtc;
- struct drm_framebuffer *fb = state->fb;
- bool is_rt_pipe;
- const struct dpu_format *fmt =
- to_dpu_format(msm_framebuffer_format(fb));
- struct dpu_hw_sspp_cfg *pipe_cfg = >pipe_cfg;
- _dpu_plane_set_scanout(plane, pstate, fb);
-
- pstate->pending = true;
-
- is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
- pstate->needs_qos_remap |= (is_rt_pipe != pdpu->is_rt_pipe);
- pdpu->is_rt_pipe = is_rt_pipe;
+ if (layout && pipe->sspp->ops.setup_sourceaddress) {
+ trace_dpu_plane_set_scanout(pipe, layout);
+ pipe->sspp->ops.setup_sourceaddress(pipe, layout);
+ }
_dpu_plane_set_qos_ctrl(plane, pipe, false, DPU_PLANE_QOS_PANIC_CTRL);
- DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " DRM_RECT_FMT
-
[Freedreno] [PATCH v3 22/27] drm/msm/dpu: rework dpu_plane_sspp_atomic_update()
Split pipe-dependent code from dpu_plane_sspp_atomic_update() into the
separate function dpu_plane_sspp_update_pipe(). This is one of
preparational steps to add r_pipe support.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 113 --
1 file changed, 63 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 0986e740b978..f94e132733f3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -404,12 +404,13 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane
*plane,
* _dpu_plane_set_ot_limit - set OT limit for the given plane
* @plane: Pointer to drm plane
* @pipe: Pointer to software pipe
- * @crtc: Pointer to drm crtc
* @pipe_cfg: Pointer to pipe configuration
+ * @frame_rate:CRTC's frame rate
*/
static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
struct dpu_sw_pipe *pipe,
- struct drm_crtc *crtc, struct dpu_hw_sspp_cfg *pipe_cfg)
+ struct dpu_hw_sspp_cfg *pipe_cfg,
+ int frame_rate)
{
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_vbif_set_ot_params ot_params;
@@ -421,7 +422,7 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
ot_params.width = drm_rect_width(_cfg->src_rect);
ot_params.height = drm_rect_height(_cfg->src_rect);
ot_params.is_wfd = !pdpu->is_rt_pipe;
- ot_params.frame_rate = drm_mode_vrefresh(>mode);
+ ot_params.frame_rate = frame_rate;
ot_params.vbif_idx = VBIF_RT;
ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
ot_params.rd = true;
@@ -457,26 +458,6 @@ static void _dpu_plane_set_qos_remap(struct drm_plane
*plane,
dpu_vbif_set_qos_remap(dpu_kms, _params);
}
-static void _dpu_plane_set_scanout(struct drm_plane *plane,
- struct dpu_plane_state *pstate,
- struct drm_framebuffer *fb)
-{
- struct dpu_plane *pdpu = to_dpu_plane(plane);
- struct dpu_kms *kms = _dpu_plane_get_kms(>base);
- struct msm_gem_address_space *aspace = kms->base.aspace;
- struct dpu_hw_fmt_layout layout;
- int ret;
-
- ret = dpu_format_populate_layout(aspace, fb, );
- if (ret)
- DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
- else if (pstate->pipe.sspp->ops.setup_sourceaddress) {
- trace_dpu_plane_set_scanout(>pipe,
- );
- pstate->pipe.sspp->ops.setup_sourceaddress(>pipe,
);
- }
-}
-
static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw,
uint32_t src_w, uint32_t src_h, uint32_t dst_w, uint32_t dst_h,
struct dpu_hw_scaler3_cfg *scale_cfg,
@@ -1102,35 +1083,25 @@ void dpu_plane_set_error(struct drm_plane *plane, bool
error)
pdpu->is_error = error;
}
-static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
+static void dpu_plane_sspp_update_pipe(struct drm_plane *plane,
+ struct dpu_sw_pipe *pipe,
+ struct dpu_hw_sspp_cfg *pipe_cfg,
+ const struct dpu_format *fmt,
+ int frame_rate,
+ struct dpu_hw_fmt_layout *layout)
{
uint32_t src_flags;
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct drm_plane_state *state = plane->state;
struct dpu_plane_state *pstate = to_dpu_plane_state(state);
- struct dpu_sw_pipe *pipe = >pipe;
- struct drm_crtc *crtc = state->crtc;
- struct drm_framebuffer *fb = state->fb;
- bool is_rt_pipe;
- const struct dpu_format *fmt =
- to_dpu_format(msm_framebuffer_format(fb));
- struct dpu_hw_sspp_cfg *pipe_cfg = >pipe_cfg;
- _dpu_plane_set_scanout(plane, pstate, fb);
-
- pstate->pending = true;
-
- is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
- pstate->needs_qos_remap |= (is_rt_pipe != pdpu->is_rt_pipe);
- pdpu->is_rt_pipe = is_rt_pipe;
+ if (layout && pipe->sspp->ops.setup_sourceaddress) {
+ trace_dpu_plane_set_scanout(pipe, layout);
+ pipe->sspp->ops.setup_sourceaddress(pipe, layout);
+ }
_dpu_plane_set_qos_ctrl(plane, pipe, false, DPU_PLANE_QOS_PANIC_CTRL);
- DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " DRM_RECT_FMT
- ", %4.4s ubwc %d\n", fb->base.id,
DRM_RECT_FP_ARG(>src),
- crtc->base.id, DRM_RECT_ARG(>dst),
- (char *)>base.pixel_format,
DPU_FORMAT_IS_UBWC(fmt));
-
/* override for color fill */
if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) {
