mbox series

[v7,00/32] drm/msm/dpu: wide planes support

Message ID 20230316161653.4106395-1-dmitry.baryshkov@linaro.org
Headers show
Series drm/msm/dpu: wide planes support | expand

Message

Dmitry Baryshkov March 16, 2023, 4:16 p.m. UTC
This patchset brings in multirect usage to support using two SSPP
rectangles for a single plane. Full virtual planes support is omitted
from this pull request, it will come later (I'm at the final stages of
polishing and testing, will be posted today).

Changes since v6:
- Really fixed line width check for UBWC formats (Abhinav)
- Also dropped R0/R1/R_MAX previously used by
  dpu_plane_validate_multirect_v2()
- Explicitly enabled SmartDMA for SC7280 following Abhinav's testing
- Reapplied Abhinav's Tested-by tags with the # sc7280 comment

Changes since v5:
- Rephrased wide planes commit message
- Fixed line width check for UBWC formats (Abhinav)
- Also drop struct dpu_multirect_plane_states, unused now

Changes since v4:
- Incorporate two fixes and two cleanup patches from Abhinav

Changes since v3:

- moved if (!pipe->sspp) checks back to the calling site, the caller
  should know if there is a backing SSPP or not.
- Restored state_idx argument of trace_dpu_crtc_setup_mixer trace point
- Removed .smart_dma_rev from dpu_caps
- Added cleaning of multirect to _dpu_plane_atomic_disable()
- Per Abhinavs request split the SmartDMA enablement patch into the
  "verified by me" and "the rest of the platforms" patches, which is not
  supposed to be merged in. Users of other platforms are supposed to
  verify multirect support on their platforms and then send patches
  enabling SmartDMA for their SoC.
- Expanded several commit messages

Changes since v2:

- Renamed dpu_hw_pipe_cfg to dpu_hw_sspp_cfg
- Added a patch to clean up src add / layout for the solid fill planes
- Fixed several comments and commit messages which caused confusion
- Added documentation for new dpu_plane_state members
- Slightly reworked dpu_plane_atomic_check() to make it more logical and obvious.

Changes since v1 (which was ages ago):
- Rebased on top of 6.2-rc1
- Dropped the controversial _dpu_crtc_blend_setup() split patch
- Renamed dpu_hw_pipe to dpu_hw_sspp
- Other misc changes

Abhinav Kumar (2):
  drm/msm/dpu: log the multirect_index in _dpu_crtc_blend_setup_pipe
  drm/msm/dpu: remove unused dpu_plane_validate_multirect_v2 function

Dmitry Baryshkov (30):
  drm/msm/dpu: rename struct dpu_hw_pipe(_cfg) to dpu_hw_sspp(_cfg)
  drm/msm/dpu: move SSPP allocation to the RM
  drm/msm/dpu: move SSPP debugfs creation to dpu_kms.c
  drm/msm/dpu: drop EAGAIN check from dpu_format_populate_layout
  drm/msm/dpu: move pipe_hw to dpu_plane_state
  drm/msm/dpu: drop dpu_plane_pipe function
  drm/msm/dpu: introduce struct dpu_sw_pipe
  drm/msm/dpu: use dpu_sw_pipe for dpu_hw_sspp callbacks
  drm/msm/dpu: pass dpu_format to _dpu_hw_sspp_setup_scaler3()
  drm/msm/dpu: clean up SRC addresses when setting up SSPP for solid
    fill
  drm/msm/dpu: move stride programming to
    dpu_hw_sspp_setup_sourceaddress
  drm/msm/dpu: remove dpu_hw_fmt_layout from struct dpu_hw_sspp_cfg
  drm/msm/dpu: rename dpu_hw_sspp_cfg to dpu_sw_pipe_cfg
  drm/msm/dpu: drop src_split and multirect check from
    dpu_crtc_atomic_check
  drm/msm/dpu: don't use unsupported blend stages
  drm/msm/dpu: move the rest of plane checks to dpu_plane_atomic_check()
  drm/msm/dpu: drop redundant plane dst check from
    dpu_crtc_atomic_check()
  drm/msm/dpu: rewrite plane's QoS-related functions to take dpu_sw_pipe
    and dpu_format
  drm/msm/dpu: make _dpu_plane_calc_clk accept mode directly
  drm/msm/dpu: add dpu_hw_sspp_cfg to dpu_plane_state
  drm/msm/dpu: simplify dpu_plane_validate_src()
  drm/msm/dpu: rework dpu_plane_sspp_atomic_update()
  drm/msm/dpu: rework dpu_plane_atomic_check()
  drm/msm/dpu: rework plane CSC setting
  drm/msm/dpu: rework static color fill code
  drm/msm/dpu: split pipe handling from _dpu_crtc_blend_setup_mixer
  drm/msm/dpu: add support for wide planes
  drm/msm/dpu: populate SmartDMA features in hw catalog
  drm/msm/dpu: enable SmartDMA for the rest of the platforms
  drm/msm/dpu: drop smart_dma_rev from dpu_caps

 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 290 ++----
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c   |  10 +-
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    |  20 +-
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h    |   2 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c   | 169 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h   | 111 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       |  18 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c     | 866 +++++++++---------
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h     |  40 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c        |  22 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h        |  12 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h     |  19 +-
 12 files changed, 718 insertions(+), 861 deletions(-)

Comments

Dmitry Baryshkov March 28, 2023, 1:32 p.m. UTC | #1
On 16/03/2023 18:16, Dmitry Baryshkov wrote:
> From: Abhinav Kumar <quic_abhinavk@quicinc.com>
> 
> Lets print the multirect_index as well in _dpu_crtc_blend_setup_pipe()
> as it will give the complete information of the sw_pipe as well.
> 
> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

For the sake of completeness:
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 90b406e409d3..508e5b950e52 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -426,12 +426,13 @@ static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc,
>   				   format->base.pixel_format,
>   				   modifier);
>   
> -	DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
> +	DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d multirect_idx %d\n",
>   			 crtc->base.id,
>   			 stage,
>   			 plane->base.id,
>   			 sspp_idx - SSPP_NONE,
> -			 state->fb ? state->fb->base.id : -1);
> +			 state->fb ? state->fb->base.id : -1,
> +			 pipe->multirect_index);
>   
>   	stage_cfg->stage[stage][stage_idx] = sspp_idx;
>   	stage_cfg->multirect_index[stage][stage_idx] = pipe->multirect_index;
Dmitry Baryshkov March 28, 2023, 10:37 p.m. UTC | #2
On Thu, 16 Mar 2023 19:16:21 +0300, Dmitry Baryshkov wrote:
> This patchset brings in multirect usage to support using two SSPP
> rectangles for a single plane. Full virtual planes support is omitted
> from this pull request, it will come later (I'm at the final stages of
> polishing and testing, will be posted today).
> 
> Changes since v6:
> - Really fixed line width check for UBWC formats (Abhinav)
> - Also dropped R0/R1/R_MAX previously used by
>   dpu_plane_validate_multirect_v2()
> - Explicitly enabled SmartDMA for SC7280 following Abhinav's testing
> - Reapplied Abhinav's Tested-by tags with the # sc7280 comment
> 
> [...]

Applied, thanks!

[01/32] drm/msm/dpu: rename struct dpu_hw_pipe(_cfg) to dpu_hw_sspp(_cfg)
        https://gitlab.freedesktop.org/lumag/msm/-/commit/995658a1c749
[02/32] drm/msm/dpu: move SSPP allocation to the RM
        https://gitlab.freedesktop.org/lumag/msm/-/commit/476754a8ac86
[03/32] drm/msm/dpu: move SSPP debugfs creation to dpu_kms.c
        https://gitlab.freedesktop.org/lumag/msm/-/commit/ff77cf2eb1a3
[04/32] drm/msm/dpu: drop EAGAIN check from dpu_format_populate_layout
        https://gitlab.freedesktop.org/lumag/msm/-/commit/e29bb8dfd072
[05/32] drm/msm/dpu: move pipe_hw to dpu_plane_state
        https://gitlab.freedesktop.org/lumag/msm/-/commit/6d8635715af1
[06/32] drm/msm/dpu: drop dpu_plane_pipe function
        https://gitlab.freedesktop.org/lumag/msm/-/commit/a2d023b21887
[07/32] drm/msm/dpu: introduce struct dpu_sw_pipe
        https://gitlab.freedesktop.org/lumag/msm/-/commit/f2f524de417a
[08/32] drm/msm/dpu: use dpu_sw_pipe for dpu_hw_sspp callbacks
        https://gitlab.freedesktop.org/lumag/msm/-/commit/0e2e459260e3
[09/32] drm/msm/dpu: pass dpu_format to _dpu_hw_sspp_setup_scaler3()
        https://gitlab.freedesktop.org/lumag/msm/-/commit/ad03f6653014
[10/32] drm/msm/dpu: clean up SRC addresses when setting up SSPP for solid fill
        https://gitlab.freedesktop.org/lumag/msm/-/commit/49f06532da0c
[11/32] drm/msm/dpu: move stride programming to dpu_hw_sspp_setup_sourceaddress
        https://gitlab.freedesktop.org/lumag/msm/-/commit/8148109600eb
[12/32] drm/msm/dpu: remove dpu_hw_fmt_layout from struct dpu_hw_sspp_cfg
        https://gitlab.freedesktop.org/lumag/msm/-/commit/3a0421182198
[13/32] drm/msm/dpu: rename dpu_hw_sspp_cfg to dpu_sw_pipe_cfg
        https://gitlab.freedesktop.org/lumag/msm/-/commit/6f32a14dcaa6
[14/32] drm/msm/dpu: drop src_split and multirect check from dpu_crtc_atomic_check
        https://gitlab.freedesktop.org/lumag/msm/-/commit/ec72f615f49b
[15/32] drm/msm/dpu: don't use unsupported blend stages
        https://gitlab.freedesktop.org/lumag/msm/-/commit/6a67280b594e
[16/32] drm/msm/dpu: move the rest of plane checks to dpu_plane_atomic_check()
        https://gitlab.freedesktop.org/lumag/msm/-/commit/274a82886182
[17/32] drm/msm/dpu: drop redundant plane dst check from dpu_crtc_atomic_check()
        https://gitlab.freedesktop.org/lumag/msm/-/commit/36ca301a498e
[18/32] drm/msm/dpu: rewrite plane's QoS-related functions to take dpu_sw_pipe and dpu_format
        https://gitlab.freedesktop.org/lumag/msm/-/commit/9a6b14e3c2d8
[19/32] drm/msm/dpu: make _dpu_plane_calc_clk accept mode directly
        https://gitlab.freedesktop.org/lumag/msm/-/commit/ddb9302ca7be
[20/32] drm/msm/dpu: add dpu_hw_sspp_cfg to dpu_plane_state
        https://gitlab.freedesktop.org/lumag/msm/-/commit/949859a56a29
[21/32] drm/msm/dpu: simplify dpu_plane_validate_src()
        https://gitlab.freedesktop.org/lumag/msm/-/commit/d92254e80244
[22/32] drm/msm/dpu: rework dpu_plane_sspp_atomic_update()
        https://gitlab.freedesktop.org/lumag/msm/-/commit/4a59602f7c6f
[23/32] drm/msm/dpu: rework dpu_plane_atomic_check()
        https://gitlab.freedesktop.org/lumag/msm/-/commit/5aa11fa2f523
[24/32] drm/msm/dpu: rework plane CSC setting
        https://gitlab.freedesktop.org/lumag/msm/-/commit/0ef79e954d75
[25/32] drm/msm/dpu: rework static color fill code
        https://gitlab.freedesktop.org/lumag/msm/-/commit/43a636a55622
[26/32] drm/msm/dpu: split pipe handling from _dpu_crtc_blend_setup_mixer
        https://gitlab.freedesktop.org/lumag/msm/-/commit/9081cb73a25d
[28/32] drm/msm/dpu: populate SmartDMA features in hw catalog
        https://gitlab.freedesktop.org/lumag/msm/-/commit/b9d4f598cb69
[30/32] drm/msm/dpu: drop smart_dma_rev from dpu_caps
        https://gitlab.freedesktop.org/lumag/msm/-/commit/a4188f96d0a0
[31/32] drm/msm/dpu: log the multirect_index in _dpu_crtc_blend_setup_pipe
        https://gitlab.freedesktop.org/lumag/msm/-/commit/a0d1028e968c
[32/32] drm/msm/dpu: remove unused dpu_plane_validate_multirect_v2 function
        https://gitlab.freedesktop.org/lumag/msm/-/commit/0a48a0014533

Best regards,
Abhinav Kumar March 30, 2023, 2:53 a.m. UTC | #3
On 3/16/2023 9:16 AM, Dmitry Baryshkov wrote:
> It is possible to use multirect feature and split source to use the SSPP
> to output two consecutive rectangles. This commit brings in this
> capability to support wider screen resolutions.
> 
> Tested-by: Abhinav Kumar <quic_abhinavk@quicinc.com> # sc7280
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  19 ++-
>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 136 +++++++++++++++++++---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h |   4 +
>   3 files changed, 142 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 217a8112f1a2..90b406e409d3 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -481,6 +481,15 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
>   					   format, fb ? fb->modifier : 0,
>   					   &pstate->pipe, 0, stage_cfg);
>   
> +		if (pstate->r_pipe.sspp) {
> +			set_bit(pstate->r_pipe.sspp->idx, fetch_active);
> +			_dpu_crtc_blend_setup_pipe(crtc, plane,
> +						   mixer, cstate->num_mixers,
> +						   pstate->stage,
> +						   format, fb ? fb->modifier : 0,
> +						   &pstate->r_pipe, 1, stage_cfg);
> +		}
> +
>   		/* blend config update */
>   		for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) {
>   			_dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate, format);
> @@ -1341,10 +1350,16 @@ static int _dpu_debugfs_status_show(struct seq_file *s, void *data)
>   		seq_printf(s, "\tdst x:%4d dst_y:%4d dst_w:%4d dst_h:%4d\n",
>   			state->crtc_x, state->crtc_y, state->crtc_w,
>   			state->crtc_h);
> -		seq_printf(s, "\tsspp:%s\n",
> +		seq_printf(s, "\tsspp[0]:%s\n",
>   			   pstate->pipe.sspp->cap->name);
> -		seq_printf(s, "\tmultirect: mode: %d index: %d\n",
> +		seq_printf(s, "\tmultirect[0]: mode: %d index: %d\n",
>   			pstate->pipe.multirect_mode, pstate->pipe.multirect_index);
> +		if (pstate->r_pipe.sspp) {
> +			seq_printf(s, "\tsspp[1]:%s\n",
> +				   pstate->r_pipe.sspp->cap->name);
> +			seq_printf(s, "\tmultirect[1]: mode: %d index: %d\n",
> +				   pstate->r_pipe.multirect_mode, pstate->r_pipe.multirect_index);
> +		}
>   
>   		seq_puts(s, "\n");
>   	}
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index f52120b05b6e..73db15d76059 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -701,6 +701,10 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
>   	/* update sspp */
>   	_dpu_plane_color_fill_pipe(pstate, &pstate->pipe, &pstate->pipe_cfg.dst_rect,
>   				   fill_color, fmt);
> +
> +	if (pstate->r_pipe.sspp)
> +		_dpu_plane_color_fill_pipe(pstate, &pstate->r_pipe, &pstate->r_pipe_cfg.dst_rect,
> +					   fill_color, fmt);
>   }
>   
>   int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane)
> @@ -959,9 +963,12 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>   	int ret = 0, min_scale;
>   	struct dpu_plane *pdpu = to_dpu_plane(plane);
>   	struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
> +	struct dpu_sw_pipe *pipe = &pstate->pipe;
> +	struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
>   	const struct drm_crtc_state *crtc_state = NULL;
>   	const struct dpu_format *fmt;
>   	struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
> +	struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
>   	struct drm_rect fb_rect = { 0 };
>   	uint32_t max_linewidth;
>   	unsigned int rotation;
> @@ -985,8 +992,11 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>   	if (!new_plane_state->visible)
>   		return 0;
>   
> -	pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
> -	pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> +	pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> +	pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> +	r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> +	r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> +	r_pipe->sspp = NULL;
>   
>   	pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
>   	if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
> @@ -1016,21 +1026,67 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>   		return -E2BIG;
>   	}
>   
> +	fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
> +
>   	max_linewidth = pdpu->catalog->caps->max_linewidth;
>   
> -	/* check decimated source width */
>   	if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) {
> -		DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n",
> -				DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
> -		return -E2BIG;
> -	}
> +		/*
> +		 * In parallel multirect case only the half of the usual width
> +		 * is supported for tiled formats. If we are here, we know that
> +		 * full width is more than max_linewidth, thus each rect is
> +		 * wider than allowed.
> +		 */
> +		if (DPU_FORMAT_IS_UBWC(fmt)) {
> +			DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u, tiled format\n",
> +					DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
> +			return -E2BIG;
> +		}

I am seeing a strange issue due to this check. Looks like Chrome 
compositor tries to do the first frame with UBWC. When I bootup with my 
DP monitor connected, it tries to use 2560x1440 with UBWC enabled but 
then fails and then uses linear format and works.

I confirmed by commenting out this check that its always UBWC.

I feel this is a compositor issue. If someone can confirm this that 
would be great because there is nothing wrong with this check.

I only want to report this because when this series is cherry-picked and 
someone tries to bootup with DP connected and it if it does hit this 
check based on which monitor they try, UBWC no longer works.

If we can move on and leave that out as compositor issue, I will add my 
R-b once the other comment is responded to.

Rest of the testing looks fine.

>   
> -	fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
> +		if (drm_rect_width(&pipe_cfg->src_rect) > 2 * max_linewidth) {
> +			DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n",
> +					DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
> +			return -E2BIG;
> +		}
>   
> -	ret = dpu_plane_atomic_check_pipe(pdpu, &pstate->pipe, pipe_cfg, fmt);
> +		if (drm_rect_width(&pipe_cfg->src_rect) != drm_rect_width(&pipe_cfg->dst_rect) ||
> +		    drm_rect_height(&pipe_cfg->src_rect) != drm_rect_height(&pipe_cfg->dst_rect) ||
> +		    (!test_bit(DPU_SSPP_SMART_DMA_V1, &pipe->sspp->cap->features) &&
> +		     !test_bit(DPU_SSPP_SMART_DMA_V2, &pipe->sspp->cap->features)) ||
> +		    DPU_FORMAT_IS_YUV(fmt)) {
> +			DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u, can't use split source\n",
> +					DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
> +			return -E2BIG;
> +		}
> +
> +		/*
> +		 * Use multirect for wide plane. We do not support dynamic
> +		 * assignment of SSPPs, so we know the configuration.
> +		 */
> +		pipe->multirect_index = DPU_SSPP_RECT_0;
> +		pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
> +
> +		r_pipe->sspp = pipe->sspp;
> +		r_pipe->multirect_index = DPU_SSPP_RECT_1;
> +		r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
> +
> +		*r_pipe_cfg = *pipe_cfg;
> +		pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 + pipe_cfg->src_rect.x2) >> 1;
> +		pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 + pipe_cfg->dst_rect.x2) >> 1;
> +		r_pipe_cfg->src_rect.x1 = pipe_cfg->src_rect.x2;
> +		r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
> +	}
> +
> +	ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt);
>   	if (ret)
>   		return ret;
>   
> +	if (r_pipe->sspp) {
> +		ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg, fmt);
> +		if (ret)
> +			return ret;
> +	}
> +
>   	supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
>   
>   	if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
> @@ -1097,8 +1153,10 @@ void dpu_plane_flush(struct drm_plane *plane)
>   	else if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG)
>   		/* force 100% alpha */
>   		_dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
> -	else
> +	else {
>   		dpu_plane_flush_csc(pdpu, &pstate->pipe);
> +		dpu_plane_flush_csc(pdpu, &pstate->r_pipe);
> +	}
>   
>   	/* flag h/w flush complete */
>   	if (plane->state)
> @@ -1210,13 +1268,14 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>   	struct drm_plane_state *state = plane->state;
>   	struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>   	struct dpu_sw_pipe *pipe = &pstate->pipe;
> +	struct dpu_sw_pipe *r_pipe = &pstate->r_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_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
> -
> +	struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
>   	struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
>   	struct msm_gem_address_space *aspace = kms->base.aspace;
>   	struct dpu_hw_fmt_layout layout;
> @@ -1244,6 +1303,12 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>   				   drm_mode_vrefresh(&crtc->mode),
>   				   layout_valid ? &layout : NULL);
>   
> +	if (r_pipe->sspp) {
> +		dpu_plane_sspp_update_pipe(plane, r_pipe, r_pipe_cfg, fmt,
> +					   drm_mode_vrefresh(&crtc->mode),
> +					   layout_valid ? &layout : NULL);
> +	}
> +
>   	if (pstate->needs_qos_remap)
>   		pstate->needs_qos_remap = false;
>   
> @@ -1251,16 +1316,31 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>   						    &crtc->mode, pipe_cfg);
>   
>   	pstate->plane_clk = _dpu_plane_calc_clk(&crtc->mode, pipe_cfg);
> +
> +	if (r_pipe->sspp) {
> +		pstate->plane_fetch_bw += _dpu_plane_calc_bw(pdpu->catalog, fmt, &crtc->mode, r_pipe_cfg);
> +
> +		pstate->plane_clk = max(pstate->plane_clk, _dpu_plane_calc_clk(&crtc->mode, r_pipe_cfg));
> +	}
>   }
>   
>   static void _dpu_plane_atomic_disable(struct drm_plane *plane)
>   {
>   	struct drm_plane_state *state = plane->state;
>   	struct dpu_plane_state *pstate = to_dpu_plane_state(state);
> +	struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
>   
>   	trace_dpu_plane_disable(DRMID(plane), false,
>   				pstate->pipe.multirect_mode);
>   
> +	if (r_pipe->sspp) {
> +		r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> +		r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> +
> +		if (r_pipe->sspp->ops.setup_multirect)
> +			r_pipe->sspp->ops.setup_multirect(r_pipe);
> +	}

Dont we need to do this even for pstate->pipe?
> +
>   	pstate->pending = true;
>   }
>   
> @@ -1293,6 +1373,9 @@ static void dpu_plane_destroy(struct drm_plane *plane)
>   		pstate = to_dpu_plane_state(plane->state);
>   		_dpu_plane_set_qos_ctrl(plane, &pstate->pipe, false, DPU_PLANE_QOS_PANIC_CTRL);
>   
> +		if (pstate->r_pipe.sspp)
> +			_dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, false, DPU_PLANE_QOS_PANIC_CTRL);
> +
>   		mutex_destroy(&pdpu->lock);
>   
>   		/* this will destroy the states as well */
> @@ -1373,12 +1456,29 @@ static void dpu_plane_atomic_print_state(struct drm_printer *p,
>   		const struct drm_plane_state *state)
>   {
>   	const struct dpu_plane_state *pstate = to_dpu_plane_state(state);
> +	const struct dpu_sw_pipe *pipe = &pstate->pipe;
> +	const struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
> +	const struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
> +	const struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
>   
>   	drm_printf(p, "\tstage=%d\n", pstate->stage);
> -	drm_printf(p, "\tsspp=%s\n", pstate->pipe.sspp->cap->name);
> -	drm_printf(p, "\tmultirect_mode=%s\n", dpu_get_multirect_mode(pstate->pipe.multirect_mode));
> -	drm_printf(p, "\tmultirect_index=%s\n",
> -		   dpu_get_multirect_index(pstate->pipe.multirect_index));
> +
> +	drm_printf(p, "\tsspp[0]=%s\n", pipe->sspp->cap->name);
> +	drm_printf(p, "\tmultirect_mode[0]=%s\n", dpu_get_multirect_mode(pipe->multirect_mode));
> +	drm_printf(p, "\tmultirect_index[0]=%s\n",
> +		   dpu_get_multirect_index(pipe->multirect_index));
> +	drm_printf(p, "\tsrc[0]=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&pipe_cfg->src_rect));
> +	drm_printf(p, "\tdst[0]=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&pipe_cfg->dst_rect));
> +
> +	if (r_pipe->sspp) {
> +		drm_printf(p, "\tsspp[1]=%s\n", r_pipe->sspp->cap->name);
> +		drm_printf(p, "\tmultirect_mode[1]=%s\n",
> +			   dpu_get_multirect_mode(r_pipe->multirect_mode));
> +		drm_printf(p, "\tmultirect_index[1]=%s\n",
> +			   dpu_get_multirect_index(r_pipe->multirect_index));
> +		drm_printf(p, "\tsrc[1]=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&r_pipe_cfg->src_rect));
> +		drm_printf(p, "\tdst[1]=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&r_pipe_cfg->dst_rect));
> +	}
>   }
>   
>   static void dpu_plane_reset(struct drm_plane *plane)
> @@ -1412,6 +1512,10 @@ static void dpu_plane_reset(struct drm_plane *plane)
>   	 * This is the place where the state is allocated, so fill it fully.
>   	 */
>   	pstate->pipe.sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe);
> +	pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
> +	pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> +
> +	pstate->r_pipe.sspp = NULL;
>   
>   	__drm_atomic_helper_plane_reset(plane, &pstate->base);
>   }
> @@ -1428,6 +1532,8 @@ void dpu_plane_danger_signal_ctrl(struct drm_plane *plane, bool enable)
>   
>   	pm_runtime_get_sync(&dpu_kms->pdev->dev);
>   	_dpu_plane_set_qos_ctrl(plane, &pstate->pipe, enable, DPU_PLANE_QOS_PANIC_CTRL);
> +	if (pstate->r_pipe.sspp)
> +		_dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, enable, DPU_PLANE_QOS_PANIC_CTRL);
>   	pm_runtime_put_sync(&dpu_kms->pdev->dev);
>   }
>   #endif
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> index 0ca9002015ff..7490ffd94d03 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> @@ -19,7 +19,9 @@
>    * @base:	base drm plane state object
>    * @aspace:	pointer to address space for input/output buffers
>    * @pipe:	software pipe description
> + * @r_pipe:	software pipe description of the second pipe
>    * @pipe_cfg:	software pipe configuration
> + * @r_pipe_cfg:	software pipe configuration for the second pipe
>    * @stage:	assigned by crtc blender
>    * @needs_qos_remap: qos remap settings need to be updated
>    * @multirect_index: index of the rectangle of SSPP
> @@ -34,7 +36,9 @@ struct dpu_plane_state {
>   	struct drm_plane_state base;
>   	struct msm_gem_address_space *aspace;
>   	struct dpu_sw_pipe pipe;
> +	struct dpu_sw_pipe r_pipe;
>   	struct dpu_sw_pipe_cfg pipe_cfg;
> +	struct dpu_sw_pipe_cfg r_pipe_cfg;
>   	enum dpu_stage stage;
>   	bool needs_qos_remap;
>   	bool pending;
Dmitry Baryshkov March 30, 2023, 3:30 a.m. UTC | #4
On 30/03/2023 05:53, Abhinav Kumar wrote:
> 
> 
> On 3/16/2023 9:16 AM, Dmitry Baryshkov wrote:
>> It is possible to use multirect feature and split source to use the SSPP
>> to output two consecutive rectangles. This commit brings in this
>> capability to support wider screen resolutions.
>>
>> Tested-by: Abhinav Kumar <quic_abhinavk@quicinc.com> # sc7280
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  19 ++-
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 136 +++++++++++++++++++---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h |   4 +
>>   3 files changed, 142 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> index 217a8112f1a2..90b406e409d3 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> @@ -481,6 +481,15 @@ static void _dpu_crtc_blend_setup_mixer(struct 
>> drm_crtc *crtc,
>>                          format, fb ? fb->modifier : 0,
>>                          &pstate->pipe, 0, stage_cfg);
>> +        if (pstate->r_pipe.sspp) {
>> +            set_bit(pstate->r_pipe.sspp->idx, fetch_active);
>> +            _dpu_crtc_blend_setup_pipe(crtc, plane,
>> +                           mixer, cstate->num_mixers,
>> +                           pstate->stage,
>> +                           format, fb ? fb->modifier : 0,
>> +                           &pstate->r_pipe, 1, stage_cfg);
>> +        }
>> +
>>           /* blend config update */
>>           for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) {
>>               _dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate, format);
>> @@ -1341,10 +1350,16 @@ static int _dpu_debugfs_status_show(struct 
>> seq_file *s, void *data)
>>           seq_printf(s, "\tdst x:%4d dst_y:%4d dst_w:%4d dst_h:%4d\n",
>>               state->crtc_x, state->crtc_y, state->crtc_w,
>>               state->crtc_h);
>> -        seq_printf(s, "\tsspp:%s\n",
>> +        seq_printf(s, "\tsspp[0]:%s\n",
>>                  pstate->pipe.sspp->cap->name);
>> -        seq_printf(s, "\tmultirect: mode: %d index: %d\n",
>> +        seq_printf(s, "\tmultirect[0]: mode: %d index: %d\n",
>>               pstate->pipe.multirect_mode, pstate->pipe.multirect_index);
>> +        if (pstate->r_pipe.sspp) {
>> +            seq_printf(s, "\tsspp[1]:%s\n",
>> +                   pstate->r_pipe.sspp->cap->name);
>> +            seq_printf(s, "\tmultirect[1]: mode: %d index: %d\n",
>> +                   pstate->r_pipe.multirect_mode, 
>> pstate->r_pipe.multirect_index);
>> +        }
>>           seq_puts(s, "\n");
>>       }
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> index f52120b05b6e..73db15d76059 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> @@ -701,6 +701,10 @@ static void _dpu_plane_color_fill(struct 
>> dpu_plane *pdpu,
>>       /* update sspp */
>>       _dpu_plane_color_fill_pipe(pstate, &pstate->pipe, 
>> &pstate->pipe_cfg.dst_rect,
>>                      fill_color, fmt);
>> +
>> +    if (pstate->r_pipe.sspp)
>> +        _dpu_plane_color_fill_pipe(pstate, &pstate->r_pipe, 
>> &pstate->r_pipe_cfg.dst_rect,
>> +                       fill_color, fmt);
>>   }
>>   int dpu_plane_validate_multirect_v2(struct 
>> dpu_multirect_plane_states *plane)
>> @@ -959,9 +963,12 @@ static int dpu_plane_atomic_check(struct 
>> drm_plane *plane,
>>       int ret = 0, min_scale;
>>       struct dpu_plane *pdpu = to_dpu_plane(plane);
>>       struct dpu_plane_state *pstate = 
>> to_dpu_plane_state(new_plane_state);
>> +    struct dpu_sw_pipe *pipe = &pstate->pipe;
>> +    struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
>>       const struct drm_crtc_state *crtc_state = NULL;
>>       const struct dpu_format *fmt;
>>       struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
>> +    struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
>>       struct drm_rect fb_rect = { 0 };
>>       uint32_t max_linewidth;
>>       unsigned int rotation;
>> @@ -985,8 +992,11 @@ static int dpu_plane_atomic_check(struct 
>> drm_plane *plane,
>>       if (!new_plane_state->visible)
>>           return 0;
>> -    pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
>> -    pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>> +    pipe->multirect_index = DPU_SSPP_RECT_SOLO;
>> +    pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>> +    r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
>> +    r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>> +    r_pipe->sspp = NULL;
>>       pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
>>       if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
>> @@ -1016,21 +1026,67 @@ static int dpu_plane_atomic_check(struct 
>> drm_plane *plane,
>>           return -E2BIG;
>>       }
>> +    fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
>> +
>>       max_linewidth = pdpu->catalog->caps->max_linewidth;
>> -    /* check decimated source width */
>>       if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) {
>> -        DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n",
>> -                DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
>> -        return -E2BIG;
>> -    }
>> +        /*
>> +         * In parallel multirect case only the half of the usual width
>> +         * is supported for tiled formats. If we are here, we know that
>> +         * full width is more than max_linewidth, thus each rect is
>> +         * wider than allowed.
>> +         */
>> +        if (DPU_FORMAT_IS_UBWC(fmt)) {
>> +            DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " 
>> line:%u, tiled format\n",
>> +                    DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
>> +            return -E2BIG;
>> +        }
> 
> I am seeing a strange issue due to this check. Looks like Chrome 
> compositor tries to do the first frame with UBWC. When I bootup with my 
> DP monitor connected, it tries to use 2560x1440 with UBWC enabled but 
> then fails and then uses linear format and works.
> 
> I confirmed by commenting out this check that its always UBWC.
> 
> I feel this is a compositor issue. If someone can confirm this that 
> would be great because there is nothing wrong with this check.

Yes, I think it is a compositor issue (or misfeature).

> 
> I only want to report this because when this series is cherry-picked and 
> someone tries to bootup with DP connected and it if it does hit this 
> check based on which monitor they try, UBWC no longer works.
> 
> If we can move on and leave that out as compositor issue, I will add my 
> R-b once the other comment is responded to.
> 
> Rest of the testing looks fine.
> 
>> -    fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
>> +        if (drm_rect_width(&pipe_cfg->src_rect) > 2 * max_linewidth) {
>> +            DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " 
>> line:%u\n",
>> +                    DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
>> +            return -E2BIG;
>> +        }
>> -    ret = dpu_plane_atomic_check_pipe(pdpu, &pstate->pipe, pipe_cfg, 
>> fmt);
>> +        if (drm_rect_width(&pipe_cfg->src_rect) != 
>> drm_rect_width(&pipe_cfg->dst_rect) ||
>> +            drm_rect_height(&pipe_cfg->src_rect) != 
>> drm_rect_height(&pipe_cfg->dst_rect) ||
>> +            (!test_bit(DPU_SSPP_SMART_DMA_V1, 
>> &pipe->sspp->cap->features) &&
>> +             !test_bit(DPU_SSPP_SMART_DMA_V2, 
>> &pipe->sspp->cap->features)) ||
>> +            DPU_FORMAT_IS_YUV(fmt)) {
>> +            DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " 
>> line:%u, can't use split source\n",
>> +                    DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
>> +            return -E2BIG;
>> +        }
>> +
>> +        /*
>> +         * Use multirect for wide plane. We do not support dynamic
>> +         * assignment of SSPPs, so we know the configuration.
>> +         */
>> +        pipe->multirect_index = DPU_SSPP_RECT_0;
>> +        pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
>> +
>> +        r_pipe->sspp = pipe->sspp;
>> +        r_pipe->multirect_index = DPU_SSPP_RECT_1;
>> +        r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
>> +
>> +        *r_pipe_cfg = *pipe_cfg;
>> +        pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 + 
>> pipe_cfg->src_rect.x2) >> 1;
>> +        pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 + 
>> pipe_cfg->dst_rect.x2) >> 1;
>> +        r_pipe_cfg->src_rect.x1 = pipe_cfg->src_rect.x2;
>> +        r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
>> +    }
>> +
>> +    ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt);
>>       if (ret)
>>           return ret;
>> +    if (r_pipe->sspp) {
>> +        ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg, 
>> fmt);
>> +        if (ret)
>> +            return ret;
>> +    }
>> +
>>       supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
>>       if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
>> @@ -1097,8 +1153,10 @@ void dpu_plane_flush(struct drm_plane *plane)
>>       else if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG)
>>           /* force 100% alpha */
>>           _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
>> -    else
>> +    else {
>>           dpu_plane_flush_csc(pdpu, &pstate->pipe);
>> +        dpu_plane_flush_csc(pdpu, &pstate->r_pipe);
>> +    }
>>       /* flag h/w flush complete */
>>       if (plane->state)
>> @@ -1210,13 +1268,14 @@ static void 
>> dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>>       struct drm_plane_state *state = plane->state;
>>       struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>>       struct dpu_sw_pipe *pipe = &pstate->pipe;
>> +    struct dpu_sw_pipe *r_pipe = &pstate->r_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_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
>> -
>> +    struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
>>       struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
>>       struct msm_gem_address_space *aspace = kms->base.aspace;
>>       struct dpu_hw_fmt_layout layout;
>> @@ -1244,6 +1303,12 @@ static void dpu_plane_sspp_atomic_update(struct 
>> drm_plane *plane)
>>                      drm_mode_vrefresh(&crtc->mode),
>>                      layout_valid ? &layout : NULL);
>> +    if (r_pipe->sspp) {
>> +        dpu_plane_sspp_update_pipe(plane, r_pipe, r_pipe_cfg, fmt,
>> +                       drm_mode_vrefresh(&crtc->mode),
>> +                       layout_valid ? &layout : NULL);
>> +    }
>> +
>>       if (pstate->needs_qos_remap)
>>           pstate->needs_qos_remap = false;
>> @@ -1251,16 +1316,31 @@ static void 
>> dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>>                               &crtc->mode, pipe_cfg);
>>       pstate->plane_clk = _dpu_plane_calc_clk(&crtc->mode, pipe_cfg);
>> +
>> +    if (r_pipe->sspp) {
>> +        pstate->plane_fetch_bw += _dpu_plane_calc_bw(pdpu->catalog, 
>> fmt, &crtc->mode, r_pipe_cfg);
>> +
>> +        pstate->plane_clk = max(pstate->plane_clk, 
>> _dpu_plane_calc_clk(&crtc->mode, r_pipe_cfg));
>> +    }
>>   }
>>   static void _dpu_plane_atomic_disable(struct drm_plane *plane)
>>   {
>>       struct drm_plane_state *state = plane->state;
>>       struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>> +    struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
>>       trace_dpu_plane_disable(DRMID(plane), false,
>>                   pstate->pipe.multirect_mode);
>> +    if (r_pipe->sspp) {
>> +        r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
>> +        r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>> +
>> +        if (r_pipe->sspp->ops.setup_multirect)
>> +            r_pipe->sspp->ops.setup_multirect(r_pipe);
>> +    }
> 
> Dont we need to do this even for pstate->pipe?

No. pstate->pipe will be set up when we call 
dpu_plane_sspp_atomic_update() next time. It doesn't seem to matter if 
the disabled SSPP is setup for REC_SOLO or REC_0. We have to ensure that 
REC_1 is disabled so that it doesn't conflict with REC_SOLO setup.

>> +
>>       pstate->pending = true;
>>   }
>> @@ -1293,6 +1373,9 @@ static void dpu_plane_destroy(struct drm_plane 
>> *plane)
>>           pstate = to_dpu_plane_state(plane->state);
>>           _dpu_plane_set_qos_ctrl(plane, &pstate->pipe, false, 
>> DPU_PLANE_QOS_PANIC_CTRL);
>> +        if (pstate->r_pipe.sspp)
>> +            _dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, false, 
>> DPU_PLANE_QOS_PANIC_CTRL);
>> +
>>           mutex_destroy(&pdpu->lock);
>>           /* this will destroy the states as well */
>> @@ -1373,12 +1456,29 @@ static void 
>> dpu_plane_atomic_print_state(struct drm_printer *p,
>>           const struct drm_plane_state *state)
>>   {
>>       const struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>> +    const struct dpu_sw_pipe *pipe = &pstate->pipe;
>> +    const struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
>> +    const struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
>> +    const struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
>>       drm_printf(p, "\tstage=%d\n", pstate->stage);
>> -    drm_printf(p, "\tsspp=%s\n", pstate->pipe.sspp->cap->name);
>> -    drm_printf(p, "\tmultirect_mode=%s\n", 
>> dpu_get_multirect_mode(pstate->pipe.multirect_mode));
>> -    drm_printf(p, "\tmultirect_index=%s\n",
>> -           dpu_get_multirect_index(pstate->pipe.multirect_index));
>> +
>> +    drm_printf(p, "\tsspp[0]=%s\n", pipe->sspp->cap->name);
>> +    drm_printf(p, "\tmultirect_mode[0]=%s\n", 
>> dpu_get_multirect_mode(pipe->multirect_mode));
>> +    drm_printf(p, "\tmultirect_index[0]=%s\n",
>> +           dpu_get_multirect_index(pipe->multirect_index));
>> +    drm_printf(p, "\tsrc[0]=" DRM_RECT_FMT "\n", 
>> DRM_RECT_ARG(&pipe_cfg->src_rect));
>> +    drm_printf(p, "\tdst[0]=" DRM_RECT_FMT "\n", 
>> DRM_RECT_ARG(&pipe_cfg->dst_rect));
>> +
>> +    if (r_pipe->sspp) {
>> +        drm_printf(p, "\tsspp[1]=%s\n", r_pipe->sspp->cap->name);
>> +        drm_printf(p, "\tmultirect_mode[1]=%s\n",
>> +               dpu_get_multirect_mode(r_pipe->multirect_mode));
>> +        drm_printf(p, "\tmultirect_index[1]=%s\n",
>> +               dpu_get_multirect_index(r_pipe->multirect_index));
>> +        drm_printf(p, "\tsrc[1]=" DRM_RECT_FMT "\n", 
>> DRM_RECT_ARG(&r_pipe_cfg->src_rect));
>> +        drm_printf(p, "\tdst[1]=" DRM_RECT_FMT "\n", 
>> DRM_RECT_ARG(&r_pipe_cfg->dst_rect));
>> +    }
>>   }
>>   static void dpu_plane_reset(struct drm_plane *plane)
>> @@ -1412,6 +1512,10 @@ static void dpu_plane_reset(struct drm_plane 
>> *plane)
>>        * This is the place where the state is allocated, so fill it 
>> fully.
>>        */
>>       pstate->pipe.sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe);
>> +    pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
>> +    pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>> +
>> +    pstate->r_pipe.sspp = NULL;
>>       __drm_atomic_helper_plane_reset(plane, &pstate->base);
>>   }
>> @@ -1428,6 +1532,8 @@ void dpu_plane_danger_signal_ctrl(struct 
>> drm_plane *plane, bool enable)
>>       pm_runtime_get_sync(&dpu_kms->pdev->dev);
>>       _dpu_plane_set_qos_ctrl(plane, &pstate->pipe, enable, 
>> DPU_PLANE_QOS_PANIC_CTRL);
>> +    if (pstate->r_pipe.sspp)
>> +        _dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, enable, 
>> DPU_PLANE_QOS_PANIC_CTRL);
>>       pm_runtime_put_sync(&dpu_kms->pdev->dev);
>>   }
>>   #endif
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> index 0ca9002015ff..7490ffd94d03 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> @@ -19,7 +19,9 @@
>>    * @base:    base drm plane state object
>>    * @aspace:    pointer to address space for input/output buffers
>>    * @pipe:    software pipe description
>> + * @r_pipe:    software pipe description of the second pipe
>>    * @pipe_cfg:    software pipe configuration
>> + * @r_pipe_cfg:    software pipe configuration for the second pipe
>>    * @stage:    assigned by crtc blender
>>    * @needs_qos_remap: qos remap settings need to be updated
>>    * @multirect_index: index of the rectangle of SSPP
>> @@ -34,7 +36,9 @@ struct dpu_plane_state {
>>       struct drm_plane_state base;
>>       struct msm_gem_address_space *aspace;
>>       struct dpu_sw_pipe pipe;
>> +    struct dpu_sw_pipe r_pipe;
>>       struct dpu_sw_pipe_cfg pipe_cfg;
>> +    struct dpu_sw_pipe_cfg r_pipe_cfg;
>>       enum dpu_stage stage;
>>       bool needs_qos_remap;
>>       bool pending;
Abhinav Kumar March 30, 2023, 4:51 a.m. UTC | #5
On 3/29/2023 8:30 PM, Dmitry Baryshkov wrote:
> On 30/03/2023 05:53, Abhinav Kumar wrote:
>>
>>
>> On 3/16/2023 9:16 AM, Dmitry Baryshkov wrote:
>>> It is possible to use multirect feature and split source to use the SSPP
>>> to output two consecutive rectangles. This commit brings in this
>>> capability to support wider screen resolutions.
>>>
>>> Tested-by: Abhinav Kumar <quic_abhinavk@quicinc.com> # sc7280
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  19 ++-
>>>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 136 +++++++++++++++++++---
>>>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h |   4 +
>>>   3 files changed, 142 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>> index 217a8112f1a2..90b406e409d3 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>> @@ -481,6 +481,15 @@ static void _dpu_crtc_blend_setup_mixer(struct 
>>> drm_crtc *crtc,
>>>                          format, fb ? fb->modifier : 0,
>>>                          &pstate->pipe, 0, stage_cfg);
>>> +        if (pstate->r_pipe.sspp) {
>>> +            set_bit(pstate->r_pipe.sspp->idx, fetch_active);
>>> +            _dpu_crtc_blend_setup_pipe(crtc, plane,
>>> +                           mixer, cstate->num_mixers,
>>> +                           pstate->stage,
>>> +                           format, fb ? fb->modifier : 0,
>>> +                           &pstate->r_pipe, 1, stage_cfg);
>>> +        }
>>> +
>>>           /* blend config update */
>>>           for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) {
>>>               _dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate, format);
>>> @@ -1341,10 +1350,16 @@ static int _dpu_debugfs_status_show(struct 
>>> seq_file *s, void *data)
>>>           seq_printf(s, "\tdst x:%4d dst_y:%4d dst_w:%4d dst_h:%4d\n",
>>>               state->crtc_x, state->crtc_y, state->crtc_w,
>>>               state->crtc_h);
>>> -        seq_printf(s, "\tsspp:%s\n",
>>> +        seq_printf(s, "\tsspp[0]:%s\n",
>>>                  pstate->pipe.sspp->cap->name);
>>> -        seq_printf(s, "\tmultirect: mode: %d index: %d\n",
>>> +        seq_printf(s, "\tmultirect[0]: mode: %d index: %d\n",
>>>               pstate->pipe.multirect_mode, 
>>> pstate->pipe.multirect_index);
>>> +        if (pstate->r_pipe.sspp) {
>>> +            seq_printf(s, "\tsspp[1]:%s\n",
>>> +                   pstate->r_pipe.sspp->cap->name);
>>> +            seq_printf(s, "\tmultirect[1]: mode: %d index: %d\n",
>>> +                   pstate->r_pipe.multirect_mode, 
>>> pstate->r_pipe.multirect_index);
>>> +        }
>>>           seq_puts(s, "\n");
>>>       }
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> index f52120b05b6e..73db15d76059 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> @@ -701,6 +701,10 @@ static void _dpu_plane_color_fill(struct 
>>> dpu_plane *pdpu,
>>>       /* update sspp */
>>>       _dpu_plane_color_fill_pipe(pstate, &pstate->pipe, 
>>> &pstate->pipe_cfg.dst_rect,
>>>                      fill_color, fmt);
>>> +
>>> +    if (pstate->r_pipe.sspp)
>>> +        _dpu_plane_color_fill_pipe(pstate, &pstate->r_pipe, 
>>> &pstate->r_pipe_cfg.dst_rect,
>>> +                       fill_color, fmt);
>>>   }
>>>   int dpu_plane_validate_multirect_v2(struct 
>>> dpu_multirect_plane_states *plane)
>>> @@ -959,9 +963,12 @@ static int dpu_plane_atomic_check(struct 
>>> drm_plane *plane,
>>>       int ret = 0, min_scale;
>>>       struct dpu_plane *pdpu = to_dpu_plane(plane);
>>>       struct dpu_plane_state *pstate = 
>>> to_dpu_plane_state(new_plane_state);
>>> +    struct dpu_sw_pipe *pipe = &pstate->pipe;
>>> +    struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
>>>       const struct drm_crtc_state *crtc_state = NULL;
>>>       const struct dpu_format *fmt;
>>>       struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
>>> +    struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
>>>       struct drm_rect fb_rect = { 0 };
>>>       uint32_t max_linewidth;
>>>       unsigned int rotation;
>>> @@ -985,8 +992,11 @@ static int dpu_plane_atomic_check(struct 
>>> drm_plane *plane,
>>>       if (!new_plane_state->visible)
>>>           return 0;
>>> -    pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
>>> -    pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>>> +    pipe->multirect_index = DPU_SSPP_RECT_SOLO;
>>> +    pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>>> +    r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
>>> +    r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>>> +    r_pipe->sspp = NULL;
>>>       pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
>>>       if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
>>> @@ -1016,21 +1026,67 @@ static int dpu_plane_atomic_check(struct 
>>> drm_plane *plane,
>>>           return -E2BIG;
>>>       }
>>> +    fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
>>> +
>>>       max_linewidth = pdpu->catalog->caps->max_linewidth;
>>> -    /* check decimated source width */
>>>       if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) {
>>> -        DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n",
>>> -                DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
>>> -        return -E2BIG;
>>> -    }
>>> +        /*
>>> +         * In parallel multirect case only the half of the usual width
>>> +         * is supported for tiled formats. If we are here, we know that
>>> +         * full width is more than max_linewidth, thus each rect is
>>> +         * wider than allowed.
>>> +         */
>>> +        if (DPU_FORMAT_IS_UBWC(fmt)) {
>>> +            DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " 
>>> line:%u, tiled format\n",
>>> +                    DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
>>> +            return -E2BIG;
>>> +        }
>>
>> I am seeing a strange issue due to this check. Looks like Chrome 
>> compositor tries to do the first frame with UBWC. When I bootup with 
>> my DP monitor connected, it tries to use 2560x1440 with UBWC enabled 
>> but then fails and then uses linear format and works.
>>
>> I confirmed by commenting out this check that its always UBWC.
>>
>> I feel this is a compositor issue. If someone can confirm this that 
>> would be great because there is nothing wrong with this check.
> 
> Yes, I think it is a compositor issue (or misfeature).
> 

Ack, I am satisfied with below response as well,

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

>>
>> I only want to report this because when this series is cherry-picked 
>> and someone tries to bootup with DP connected and it if it does hit 
>> this check based on which monitor they try, UBWC no longer works.
>>
>> If we can move on and leave that out as compositor issue, I will add 
>> my R-b once the other comment is responded to.
>>
>> Rest of the testing looks fine.
>>
>>> -    fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
>>> +        if (drm_rect_width(&pipe_cfg->src_rect) > 2 * max_linewidth) {
>>> +            DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " 
>>> line:%u\n",
>>> +                    DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
>>> +            return -E2BIG;
>>> +        }
>>> -    ret = dpu_plane_atomic_check_pipe(pdpu, &pstate->pipe, pipe_cfg, 
>>> fmt);
>>> +        if (drm_rect_width(&pipe_cfg->src_rect) != 
>>> drm_rect_width(&pipe_cfg->dst_rect) ||
>>> +            drm_rect_height(&pipe_cfg->src_rect) != 
>>> drm_rect_height(&pipe_cfg->dst_rect) ||
>>> +            (!test_bit(DPU_SSPP_SMART_DMA_V1, 
>>> &pipe->sspp->cap->features) &&
>>> +             !test_bit(DPU_SSPP_SMART_DMA_V2, 
>>> &pipe->sspp->cap->features)) ||
>>> +            DPU_FORMAT_IS_YUV(fmt)) {
>>> +            DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " 
>>> line:%u, can't use split source\n",
>>> +                    DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
>>> +            return -E2BIG;
>>> +        }
>>> +
>>> +        /*
>>> +         * Use multirect for wide plane. We do not support dynamic
>>> +         * assignment of SSPPs, so we know the configuration.
>>> +         */
>>> +        pipe->multirect_index = DPU_SSPP_RECT_0;
>>> +        pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
>>> +
>>> +        r_pipe->sspp = pipe->sspp;
>>> +        r_pipe->multirect_index = DPU_SSPP_RECT_1;
>>> +        r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
>>> +
>>> +        *r_pipe_cfg = *pipe_cfg;
>>> +        pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 + 
>>> pipe_cfg->src_rect.x2) >> 1;
>>> +        pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 + 
>>> pipe_cfg->dst_rect.x2) >> 1;
>>> +        r_pipe_cfg->src_rect.x1 = pipe_cfg->src_rect.x2;
>>> +        r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
>>> +    }
>>> +
>>> +    ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt);
>>>       if (ret)
>>>           return ret;
>>> +    if (r_pipe->sspp) {
>>> +        ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg, 
>>> fmt);
>>> +        if (ret)
>>> +            return ret;
>>> +    }
>>> +
>>>       supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
>>>       if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
>>> @@ -1097,8 +1153,10 @@ void dpu_plane_flush(struct drm_plane *plane)
>>>       else if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG)
>>>           /* force 100% alpha */
>>>           _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
>>> -    else
>>> +    else {
>>>           dpu_plane_flush_csc(pdpu, &pstate->pipe);
>>> +        dpu_plane_flush_csc(pdpu, &pstate->r_pipe);
>>> +    }
>>>       /* flag h/w flush complete */
>>>       if (plane->state)
>>> @@ -1210,13 +1268,14 @@ static void 
>>> dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>>>       struct drm_plane_state *state = plane->state;
>>>       struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>>>       struct dpu_sw_pipe *pipe = &pstate->pipe;
>>> +    struct dpu_sw_pipe *r_pipe = &pstate->r_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_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
>>> -
>>> +    struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
>>>       struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
>>>       struct msm_gem_address_space *aspace = kms->base.aspace;
>>>       struct dpu_hw_fmt_layout layout;
>>> @@ -1244,6 +1303,12 @@ static void 
>>> dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>>>                      drm_mode_vrefresh(&crtc->mode),
>>>                      layout_valid ? &layout : NULL);
>>> +    if (r_pipe->sspp) {
>>> +        dpu_plane_sspp_update_pipe(plane, r_pipe, r_pipe_cfg, fmt,
>>> +                       drm_mode_vrefresh(&crtc->mode),
>>> +                       layout_valid ? &layout : NULL);
>>> +    }
>>> +
>>>       if (pstate->needs_qos_remap)
>>>           pstate->needs_qos_remap = false;
>>> @@ -1251,16 +1316,31 @@ static void 
>>> dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>>>                               &crtc->mode, pipe_cfg);
>>>       pstate->plane_clk = _dpu_plane_calc_clk(&crtc->mode, pipe_cfg);
>>> +
>>> +    if (r_pipe->sspp) {
>>> +        pstate->plane_fetch_bw += _dpu_plane_calc_bw(pdpu->catalog, 
>>> fmt, &crtc->mode, r_pipe_cfg);
>>> +
>>> +        pstate->plane_clk = max(pstate->plane_clk, 
>>> _dpu_plane_calc_clk(&crtc->mode, r_pipe_cfg));
>>> +    }
>>>   }
>>>   static void _dpu_plane_atomic_disable(struct drm_plane *plane)
>>>   {
>>>       struct drm_plane_state *state = plane->state;
>>>       struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>>> +    struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
>>>       trace_dpu_plane_disable(DRMID(plane), false,
>>>                   pstate->pipe.multirect_mode);
>>> +    if (r_pipe->sspp) {
>>> +        r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
>>> +        r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>>> +
>>> +        if (r_pipe->sspp->ops.setup_multirect)
>>> +            r_pipe->sspp->ops.setup_multirect(r_pipe);
>>> +    }
>>
>> Dont we need to do this even for pstate->pipe?
> 
> No. pstate->pipe will be set up when we call 
> dpu_plane_sspp_atomic_update() next time. It doesn't seem to matter if 
> the disabled SSPP is setup for REC_SOLO or REC_0. We have to ensure that 
> REC_1 is disabled so that it doesn't conflict with REC_SOLO setup.
> 
>>> +
>>>       pstate->pending = true;
>>>   }
>>> @@ -1293,6 +1373,9 @@ static void dpu_plane_destroy(struct drm_plane 
>>> *plane)
>>>           pstate = to_dpu_plane_state(plane->state);
>>>           _dpu_plane_set_qos_ctrl(plane, &pstate->pipe, false, 
>>> DPU_PLANE_QOS_PANIC_CTRL);
>>> +        if (pstate->r_pipe.sspp)
>>> +            _dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, false, 
>>> DPU_PLANE_QOS_PANIC_CTRL);
>>> +
>>>           mutex_destroy(&pdpu->lock);
>>>           /* this will destroy the states as well */
>>> @@ -1373,12 +1456,29 @@ static void 
>>> dpu_plane_atomic_print_state(struct drm_printer *p,
>>>           const struct drm_plane_state *state)
>>>   {
>>>       const struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>>> +    const struct dpu_sw_pipe *pipe = &pstate->pipe;
>>> +    const struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
>>> +    const struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
>>> +    const struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
>>>       drm_printf(p, "\tstage=%d\n", pstate->stage);
>>> -    drm_printf(p, "\tsspp=%s\n", pstate->pipe.sspp->cap->name);
>>> -    drm_printf(p, "\tmultirect_mode=%s\n", 
>>> dpu_get_multirect_mode(pstate->pipe.multirect_mode));
>>> -    drm_printf(p, "\tmultirect_index=%s\n",
>>> -           dpu_get_multirect_index(pstate->pipe.multirect_index));
>>> +
>>> +    drm_printf(p, "\tsspp[0]=%s\n", pipe->sspp->cap->name);
>>> +    drm_printf(p, "\tmultirect_mode[0]=%s\n", 
>>> dpu_get_multirect_mode(pipe->multirect_mode));
>>> +    drm_printf(p, "\tmultirect_index[0]=%s\n",
>>> +           dpu_get_multirect_index(pipe->multirect_index));
>>> +    drm_printf(p, "\tsrc[0]=" DRM_RECT_FMT "\n", 
>>> DRM_RECT_ARG(&pipe_cfg->src_rect));
>>> +    drm_printf(p, "\tdst[0]=" DRM_RECT_FMT "\n", 
>>> DRM_RECT_ARG(&pipe_cfg->dst_rect));
>>> +
>>> +    if (r_pipe->sspp) {
>>> +        drm_printf(p, "\tsspp[1]=%s\n", r_pipe->sspp->cap->name);
>>> +        drm_printf(p, "\tmultirect_mode[1]=%s\n",
>>> +               dpu_get_multirect_mode(r_pipe->multirect_mode));
>>> +        drm_printf(p, "\tmultirect_index[1]=%s\n",
>>> +               dpu_get_multirect_index(r_pipe->multirect_index));
>>> +        drm_printf(p, "\tsrc[1]=" DRM_RECT_FMT "\n", 
>>> DRM_RECT_ARG(&r_pipe_cfg->src_rect));
>>> +        drm_printf(p, "\tdst[1]=" DRM_RECT_FMT "\n", 
>>> DRM_RECT_ARG(&r_pipe_cfg->dst_rect));
>>> +    }
>>>   }
>>>   static void dpu_plane_reset(struct drm_plane *plane)
>>> @@ -1412,6 +1512,10 @@ static void dpu_plane_reset(struct drm_plane 
>>> *plane)
>>>        * This is the place where the state is allocated, so fill it 
>>> fully.
>>>        */
>>>       pstate->pipe.sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe);
>>> +    pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
>>> +    pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>>> +
>>> +    pstate->r_pipe.sspp = NULL;
>>>       __drm_atomic_helper_plane_reset(plane, &pstate->base);
>>>   }
>>> @@ -1428,6 +1532,8 @@ void dpu_plane_danger_signal_ctrl(struct 
>>> drm_plane *plane, bool enable)
>>>       pm_runtime_get_sync(&dpu_kms->pdev->dev);
>>>       _dpu_plane_set_qos_ctrl(plane, &pstate->pipe, enable, 
>>> DPU_PLANE_QOS_PANIC_CTRL);
>>> +    if (pstate->r_pipe.sspp)
>>> +        _dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, enable, 
>>> DPU_PLANE_QOS_PANIC_CTRL);
>>>       pm_runtime_put_sync(&dpu_kms->pdev->dev);
>>>   }
>>>   #endif
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h 
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>>> index 0ca9002015ff..7490ffd94d03 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>>> @@ -19,7 +19,9 @@
>>>    * @base:    base drm plane state object
>>>    * @aspace:    pointer to address space for input/output buffers
>>>    * @pipe:    software pipe description
>>> + * @r_pipe:    software pipe description of the second pipe
>>>    * @pipe_cfg:    software pipe configuration
>>> + * @r_pipe_cfg:    software pipe configuration for the second pipe
>>>    * @stage:    assigned by crtc blender
>>>    * @needs_qos_remap: qos remap settings need to be updated
>>>    * @multirect_index: index of the rectangle of SSPP
>>> @@ -34,7 +36,9 @@ struct dpu_plane_state {
>>>       struct drm_plane_state base;
>>>       struct msm_gem_address_space *aspace;
>>>       struct dpu_sw_pipe pipe;
>>> +    struct dpu_sw_pipe r_pipe;
>>>       struct dpu_sw_pipe_cfg pipe_cfg;
>>> +    struct dpu_sw_pipe_cfg r_pipe_cfg;
>>>       enum dpu_stage stage;
>>>       bool needs_qos_remap;
>>>       bool pending;
>