From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1F7E0C433F5 for ; Fri, 6 May 2022 21:39:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1444549AbiEFVnh (ORCPT ); Fri, 6 May 2022 17:43:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34336 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1352550AbiEFVng (ORCPT ); Fri, 6 May 2022 17:43:36 -0400 Received: from mail-lj1-x22a.google.com (mail-lj1-x22a.google.com [IPv6:2a00:1450:4864:20::22a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 95B7F6FA1B for ; Fri, 6 May 2022 14:39:50 -0700 (PDT) Received: by mail-lj1-x22a.google.com with SMTP id q14so10695659ljc.12 for ; Fri, 06 May 2022 14:39:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=mjyPLoeFd41X0tFwREhvBCuwQ48xk8EujtU9e1mtlaI=; b=gFqoQvJBdfNjDyqcytFyAcXnlGpr7/lJmfI3biiOnf2jzy7/ucqH/gBhc/hnyJ65cI p01dvi72NZqS21EBGroLmif+5l99BDbeAvSLbNUnGw1QUS9jOhPWrZ8J4b3jO1lTBC9s Q6JvIJnVRBtTfeBt9RzkUy2mTNbp/obYS3CMmTsUDuBRrWviMOcjP+XeEi4ME5dkL/kd idIvToj7PSsvG41n0JiCZiOhuqiUQ0euH2/QZIR9G3dbE8Y50MHVdVrITOy19n/r008A 0mtJlksExWBK/AXwaiewGTY794idiUr8gUvP26x1edjKbx89yf8E+Lj2tEDuzdWqWpfg 48Rg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=mjyPLoeFd41X0tFwREhvBCuwQ48xk8EujtU9e1mtlaI=; b=fwVnKXpmHZU8Q8zkv1F4uzhKP8EbMfemjocZncExJtoqd6+9Ns8AyUNrb2l0rQMuja FFCVDxKJK5lhkGai6kMZQE67Ezmh4T/xL2Q0LOlpVcPKTMwjuGRXAlYNokpv71xBdXyk Q9LfCG53IQxvtHXHMB8xbw0kHvySO9TTnQl2SFgTGiT9TZs3rXepXHw30F7ypNICB/OG u9BU6d0hlk6asTGPVWsw8nRG9QRfW4x0gQOUqayz9Rt3i5oYn+2cX5fpXL4ae2GIOTKS hTkgRdSe8hMXpwuwLzAM+XiqH+wu7eWmo3AuALT+e249CEdS9KWYg7kN4cD5pH7xTuBf hPYQ== X-Gm-Message-State: AOAM530r6okKSlqp1DY7JFRy3CwcghKKut065iFXgkL0o3Js9/4TGb41 yEz5PN8q8HY9d7po6zxvygpwQw== X-Google-Smtp-Source: ABdhPJxPrngsCwhHYT+sxAbFIGDfeg+SuBoZOqu9DXj1yosZVK1L20aSj3un+HhoFcE1VWxoDiwhbA== X-Received: by 2002:a2e:150e:0:b0:24a:c41a:a3e4 with SMTP id s14-20020a2e150e000000b0024ac41aa3e4mr3149688ljd.371.1651873188855; Fri, 06 May 2022 14:39:48 -0700 (PDT) Received: from [192.168.1.211] ([37.153.55.125]) by smtp.gmail.com with ESMTPSA id c2-20020a05651200c200b0047255d211ebsm823122lfp.282.2022.05.06.14.39.48 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 06 May 2022 14:39:48 -0700 (PDT) Message-ID: <27cccd15-080e-0610-09e0-e0cfc3b85f55@linaro.org> Date: Sat, 7 May 2022 00:39:47 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.1 Subject: Re: [PATCH 10/25] drm/msm/dpu: introduce struct dpu_sw_pipe Content-Language: en-GB To: Abhinav Kumar , Bjorn Andersson , Rob Clark , Sean Paul Cc: Stephen Boyd , David Airlie , Daniel Vetter , linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org References: <20220209172520.3719906-1-dmitry.baryshkov@linaro.org> <20220209172520.3719906-11-dmitry.baryshkov@linaro.org> <667a1dbe-5640-a92e-a9f8-a55935005083@quicinc.com> From: Dmitry Baryshkov In-Reply-To: <667a1dbe-5640-a92e-a9f8-a55935005083@quicinc.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org On 07/05/2022 00:30, Abhinav Kumar wrote: > > > On 2/9/2022 9:25 AM, Dmitry Baryshkov wrote: >> Wrap SSPP and multirect index/mode into a single structure that >> represents software view on the pipe used. >> >> Signed-off-by: Dmitry Baryshkov >> --- >>   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    |   8 +- >>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h |  16 ++- >>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 136 ++++++++++---------- >>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |   4 +- >>   4 files changed, 86 insertions(+), 78 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> index ada7d5750536..751c64012058 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> @@ -441,14 +441,14 @@ static void _dpu_crtc_blend_setup_ctl(struct >> drm_crtc *crtc) >>           pstate = to_dpu_plane_state(state); >> -        sspp_idx = pstate->pipe_hw->idx; >> +        sspp_idx = pstate->pipe.sspp->idx; >>           set_bit(sspp_idx, fetch_active); >>           stage_idx = zpos_cnt[pstate->stage]++; >>           stage_cfg.stage[pstate->stage][stage_idx] = >>                       sspp_idx; >>           stage_cfg.multirect_index[pstate->stage][stage_idx] = >> -                    pstate->multirect_index; >> +                    pstate->pipe.multirect_index; >>           /* blend config update */ >>           for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) >> @@ -1118,7 +1118,7 @@ static int dpu_crtc_atomic_check(struct drm_crtc >> *crtc, >>           pstates[cnt].dpu_pstate = to_dpu_plane_state(pstate); >>           pstates[cnt].drm_pstate = pstate; >>           pstates[cnt].stage = pstate->normalized_zpos; >> -        pstates[cnt].pipe_id = to_dpu_plane_state(pstate)->pipe_hw->idx; >> +        pstates[cnt].pipe_id = >> to_dpu_plane_state(pstate)->pipe.sspp->idx; >>           if (pipe_staged[pstates[cnt].pipe_id]) { >>               multirect_plane[multirect_count].r0 = >> @@ -1389,7 +1389,7 @@ static int _dpu_debugfs_status_show(struct >> seq_file *s, void *data) >>               state->crtc_x, state->crtc_y, state->crtc_w, >>               state->crtc_h); >>           seq_printf(s, "\tmultirect: mode: %d index: %d\n", >> -            pstate->multirect_mode, pstate->multirect_index); >> +            pstate->pipe.multirect_mode, pstate->pipe.multirect_index); >>           seq_puts(s, "\n"); >>       } >> 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 674f311f99b4..0af2bc6e5df8 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h >> @@ -159,15 +159,11 @@ struct dpu_hw_pixel_ext { >>    * @src_rect:  src ROI, caller takes into account the different >> operations >>    *             such as decimation, flip etc to program this field >>    * @dest_rect: destination ROI. >> - * @index:     index of the rectangle of SSPP >> - * @mode:      parallel or time multiplex multirect mode >>    */ >>   struct dpu_hw_pipe_cfg { >>       struct dpu_hw_fmt_layout layout; >>       struct drm_rect src_rect; >>       struct drm_rect dst_rect; >> -    enum dpu_sspp_multirect_index index; >> -    enum dpu_sspp_multirect_mode mode; >>   }; >>   /** >> @@ -218,6 +214,18 @@ struct dpu_hw_pipe_ts_cfg { >>       u64 time; >>   }; >> +/** >> + * struct dpu_sw_pipe - software pipe description >> + * @sspp:      backing SSPP pipe >> + * @index:     index of the rectangle of SSPP >> + * @mode:      parallel or time multiplex multirect mode >> + */ >> +struct dpu_sw_pipe { >> +    struct dpu_hw_pipe *sspp; >> +    enum dpu_sspp_multirect_index multirect_index; >> +    enum dpu_sspp_multirect_mode multirect_mode; >> +}; >> + >>   /** >>    * struct dpu_hw_sspp_ops - interface to the SSPP Hw driver functions >>    * Caller must call the init function to get the pipe context for >> each pipe >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> index d1f9b4bc10ac..51b5e8a3182b 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> @@ -247,7 +247,7 @@ static int _dpu_plane_calc_fill_level(struct >> drm_plane *plane, >>                   ((src_width + 32) * fmt->bpp); >>           } >>       } else { >> -        if (pstate->multirect_mode == DPU_SSPP_MULTIRECT_PARALLEL) { >> +        if (pstate->pipe.multirect_mode == >> DPU_SSPP_MULTIRECT_PARALLEL) { >>               total_fl = (fixed_buff_size / 2) * 2 / >>                   ((src_width + 32) * fmt->bpp); >>           } else { >> @@ -331,7 +331,7 @@ static void _dpu_plane_set_qos_lut(struct >> drm_plane *plane, >>               fmt ? (char *)&fmt->base.pixel_format : NULL, >>               pdpu->is_rt_pipe, total_fl, qos_lut); >> -    pstate->pipe_hw->ops.setup_creq_lut(pstate->pipe_hw, qos_lut); >> +    pstate->pipe.sspp->ops.setup_creq_lut(pstate->pipe.sspp, qos_lut); >>   } >>   /** >> @@ -383,7 +383,7 @@ static void _dpu_plane_set_danger_lut(struct >> drm_plane *plane, >>           danger_lut, >>           safe_lut); >> -    pstate->pipe_hw->ops.setup_danger_safe_lut(pstate->pipe_hw, >> +    pstate->pipe.sspp->ops.setup_danger_safe_lut(pstate->pipe.sspp, >>               danger_lut, safe_lut); >>   } >> @@ -403,9 +403,9 @@ static void _dpu_plane_set_qos_ctrl(struct >> drm_plane *plane, >>       memset(&pipe_qos_cfg, 0, sizeof(pipe_qos_cfg)); >>       if (flags & DPU_PLANE_QOS_VBLANK_CTRL) { >> -        pipe_qos_cfg.creq_vblank = >> pstate->pipe_hw->cap->sblk->creq_vblank; >> +        pipe_qos_cfg.creq_vblank = >> pstate->pipe.sspp->cap->sblk->creq_vblank; >>           pipe_qos_cfg.danger_vblank = >> -                pstate->pipe_hw->cap->sblk->danger_vblank; >> +                pstate->pipe.sspp->cap->sblk->danger_vblank; >>           pipe_qos_cfg.vblank_en = enable; >>       } >> @@ -431,7 +431,7 @@ static void _dpu_plane_set_qos_ctrl(struct >> drm_plane *plane, >>           pipe_qos_cfg.danger_vblank, >>           pdpu->is_rt_pipe); >> -    pstate->pipe_hw->ops.setup_qos_ctrl(pstate->pipe_hw, >> +    pstate->pipe.sspp->ops.setup_qos_ctrl(pstate->pipe.sspp, >>               &pipe_qos_cfg); >>   } >> @@ -450,14 +450,14 @@ static void _dpu_plane_set_ot_limit(struct >> drm_plane *plane, >>       struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); >>       memset(&ot_params, 0, sizeof(ot_params)); >> -    ot_params.xin_id = pstate->pipe_hw->cap->xin_id; >> -    ot_params.num = pstate->pipe_hw->idx - SSPP_NONE; >> +    ot_params.xin_id = pstate->pipe.sspp->cap->xin_id; >> +    ot_params.num = pstate->pipe.sspp->idx - SSPP_NONE; >>       ot_params.width = drm_rect_width(&pipe_cfg->src_rect); >>       ot_params.height = drm_rect_height(&pipe_cfg->src_rect); >>       ot_params.is_wfd = !pdpu->is_rt_pipe; >>       ot_params.frame_rate = drm_mode_vrefresh(&crtc->mode); >>       ot_params.vbif_idx = VBIF_RT; >> -    ot_params.clk_ctrl = pstate->pipe_hw->cap->clk_ctrl; >> +    ot_params.clk_ctrl = pstate->pipe.sspp->cap->clk_ctrl; >>       ot_params.rd = true; >>       dpu_vbif_set_ot_limit(dpu_kms, &ot_params); >> @@ -476,9 +476,9 @@ static void _dpu_plane_set_qos_remap(struct >> drm_plane *plane) >>       memset(&qos_params, 0, sizeof(qos_params)); >>       qos_params.vbif_idx = VBIF_RT; >> -    qos_params.clk_ctrl = pstate->pipe_hw->cap->clk_ctrl; >> -    qos_params.xin_id = pstate->pipe_hw->cap->xin_id; >> -    qos_params.num = pstate->pipe_hw->idx - SSPP_VIG0; >> +    qos_params.clk_ctrl = pstate->pipe.sspp->cap->clk_ctrl; >> +    qos_params.xin_id = pstate->pipe.sspp->cap->xin_id; >> +    qos_params.num = pstate->pipe.sspp->idx - SSPP_VIG0; >>       qos_params.is_rt = pdpu->is_rt_pipe; >>       DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d, >> clk_ctrl:%d\n", >> @@ -505,12 +505,12 @@ static void _dpu_plane_set_scanout(struct >> drm_plane *plane, >>           DPU_DEBUG_PLANE(pdpu, "not updating same src addrs\n"); >>       else if (ret) >>           DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", >> ret); >> -    else if (pstate->pipe_hw->ops.setup_sourceaddress) { >> -        trace_dpu_plane_set_scanout(pstate->pipe_hw->idx, >> +    else if (pstate->pipe.sspp->ops.setup_sourceaddress) { >> +        trace_dpu_plane_set_scanout(pstate->pipe.sspp->idx, >>                           &pipe_cfg->layout, >> -                        pstate->multirect_index); >> -        pstate->pipe_hw->ops.setup_sourceaddress(pstate->pipe_hw, >> pipe_cfg, >> -                        pstate->multirect_index); >> +                        pstate->pipe.multirect_index); >> +        pstate->pipe.sspp->ops.setup_sourceaddress(pstate->pipe.sspp, >> pipe_cfg, >> +                        pstate->pipe.multirect_index); >>       } >>   } >> @@ -553,7 +553,7 @@ static void _dpu_plane_setup_scaler3(struct >> dpu_plane *pdpu, >>               scale_cfg->src_height[i] /= chroma_subsmpl_v; >>           } >> -        if (pstate->pipe_hw->cap->features & >> +        if (pstate->pipe.sspp->cap->features & >>               BIT(DPU_SSPP_SCALER_QSEED4)) { >>               scale_cfg->preload_x[i] = DPU_QSEED4_DEFAULT_PRELOAD_H; >>               scale_cfg->preload_y[i] = DPU_QSEED4_DEFAULT_PRELOAD_V; >> @@ -624,7 +624,7 @@ static const struct dpu_csc_cfg >> *_dpu_plane_get_csc(struct dpu_plane *pdpu, cons >>       if (!DPU_FORMAT_IS_YUV(fmt)) >>           return NULL; >> -    if (BIT(DPU_SSPP_CSC_10BIT) & pstate->pipe_hw->cap->features) >> +    if (BIT(DPU_SSPP_CSC_10BIT) & pstate->pipe.sspp->cap->features) >>           csc_ptr = &dpu_csc10_YUV2RGB_601L; >>       else >>           csc_ptr = &dpu_csc_YUV2RGB_601L; >> @@ -659,8 +659,8 @@ static void _dpu_plane_setup_scaler(struct >> dpu_plane *pdpu, >>               &scaler3_cfg, &pixel_ext, fmt, >>               info->hsub, info->vsub); >> -    if (pstate->pipe_hw->ops.setup_pe) >> -        pstate->pipe_hw->ops.setup_pe(pstate->pipe_hw, >> +    if (pstate->pipe.sspp->ops.setup_pe) >> +        pstate->pipe.sspp->ops.setup_pe(pstate->pipe.sspp, >>                   &pixel_ext); >>       /** >> @@ -668,9 +668,9 @@ static void _dpu_plane_setup_scaler(struct >> dpu_plane *pdpu, >>        * bypassed. Still we need to update alpha and bitwidth >>        * ONLY for RECT0 >>        */ >> -    if (pstate->pipe_hw->ops.setup_scaler && >> -            pstate->multirect_index != DPU_SSPP_RECT_1) >> -        pstate->pipe_hw->ops.setup_scaler(pstate->pipe_hw, >> +    if (pstate->pipe.sspp->ops.setup_scaler && >> +            pstate->pipe.multirect_index != DPU_SSPP_RECT_1) >> +        pstate->pipe.sspp->ops.setup_scaler(pstate->pipe.sspp, >>                   pipe_cfg, >>                   &scaler3_cfg); >>   } >> @@ -699,10 +699,10 @@ static int _dpu_plane_color_fill(struct >> dpu_plane *pdpu, >>       fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR8888); >>       /* update sspp */ >> -    if (fmt && pstate->pipe_hw->ops.setup_solidfill) { >> -        pstate->pipe_hw->ops.setup_solidfill(pstate->pipe_hw, >> +    if (fmt && pstate->pipe.sspp->ops.setup_solidfill) { >> +        pstate->pipe.sspp->ops.setup_solidfill(pstate->pipe.sspp, >>                   (color & 0xFFFFFF) | ((alpha & 0xFF) << 24), >> -                pstate->multirect_index); >> +                pstate->pipe.multirect_index); >>           /* override scaler/decimation if solid fill */ >>           pipe_cfg.dst_rect = pstate->base.dst; >> @@ -714,15 +714,15 @@ static int _dpu_plane_color_fill(struct >> dpu_plane *pdpu, >>           pipe_cfg.src_rect.y2 = >>               drm_rect_height(&pipe_cfg.dst_rect); >> -        if (pstate->pipe_hw->ops.setup_format) >> -            pstate->pipe_hw->ops.setup_format(pstate->pipe_hw, >> +        if (pstate->pipe.sspp->ops.setup_format) >> +            pstate->pipe.sspp->ops.setup_format(pstate->pipe.sspp, >>                       fmt, DPU_SSPP_SOLID_FILL, >> -                    pstate->multirect_index); >> +                    pstate->pipe.multirect_index); >> -        if (pstate->pipe_hw->ops.setup_rects) >> -            pstate->pipe_hw->ops.setup_rects(pstate->pipe_hw, >> +        if (pstate->pipe.sspp->ops.setup_rects) >> +            pstate->pipe.sspp->ops.setup_rects(pstate->pipe.sspp, >>                       &pipe_cfg, >> -                    pstate->multirect_index); >> +                    pstate->pipe.multirect_index); >>           _dpu_plane_setup_scaler(pdpu, pstate, fmt, true, &pipe_cfg); >>       } >> @@ -734,8 +734,8 @@ void dpu_plane_clear_multirect(const struct >> drm_plane_state *drm_state) >>   { >>       struct dpu_plane_state *pstate = to_dpu_plane_state(drm_state); >> -    pstate->multirect_index = DPU_SSPP_RECT_SOLO; >> -    pstate->multirect_mode = DPU_SSPP_MULTIRECT_NONE; >> +    pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO; >> +    pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE; >>   } >>   int dpu_plane_validate_multirect_v2(struct >> dpu_multirect_plane_states *plane) >> @@ -817,8 +817,8 @@ int dpu_plane_validate_multirect_v2(struct >> dpu_multirect_plane_states *plane) >>       /* Prefer PARALLEL FETCH Mode over TIME_MX Mode */ >>       if (parallel_fetch_qualified) { >> -        pstate[R0]->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL; >> -        pstate[R1]->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL; >> +        pstate[R0]->pipe.multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL; >> +        pstate[R1]->pipe.multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL; >>           goto done; >>       } >> @@ -828,8 +828,8 @@ int dpu_plane_validate_multirect_v2(struct >> dpu_multirect_plane_states *plane) >>       if (dst[R1].y1 >= dst[R0].y2 + buffer_lines || >>           dst[R0].y1 >= dst[R1].y2 + buffer_lines) { >> -        pstate[R0]->multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX; >> -        pstate[R1]->multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX; >> +        pstate[R0]->pipe.multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX; >> +        pstate[R1]->pipe.multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX; >>       } else { >>           DPU_ERROR( >>               "No multirect mode possible for the planes (%d - %d)\n", >> @@ -839,13 +839,13 @@ int dpu_plane_validate_multirect_v2(struct >> dpu_multirect_plane_states *plane) >>       } >>   done: >> -    pstate[R0]->multirect_index = DPU_SSPP_RECT_0; >> -    pstate[R1]->multirect_index = DPU_SSPP_RECT_1; >> +    pstate[R0]->pipe.multirect_index = DPU_SSPP_RECT_0; >> +    pstate[R1]->pipe.multirect_index = DPU_SSPP_RECT_1; >>       DPU_DEBUG_PLANE(dpu_plane[R0], "R0: %d - %d\n", >> -        pstate[R0]->multirect_mode, pstate[R0]->multirect_index); >> +        pstate[R0]->pipe.multirect_mode, >> pstate[R0]->pipe.multirect_index); >>       DPU_DEBUG_PLANE(dpu_plane[R1], "R1: %d - %d\n", >> -        pstate[R1]->multirect_mode, pstate[R1]->multirect_index); >> +        pstate[R1]->pipe.multirect_mode, >> pstate[R1]->pipe.multirect_index); >>       return 0; >>   } >> @@ -937,6 +937,7 @@ 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; >>       const struct drm_crtc_state *crtc_state = NULL; >>       const struct dpu_format *fmt; >>       struct drm_rect src, dst, fb_rect = { 0 }; >> @@ -946,10 +947,10 @@ static int dpu_plane_atomic_check(struct >> drm_plane *plane, >>           crtc_state = drm_atomic_get_new_crtc_state(state, >>                                  new_plane_state->crtc); >> -    min_scale = FRAC_16_16(1, pstate->pipe_hw->cap->sblk->maxupscale); >> +    min_scale = FRAC_16_16(1, pipe->sspp->cap->sblk->maxupscale); >>       ret = drm_atomic_helper_check_plane_state(new_plane_state, >> crtc_state, >>                             min_scale, >> -                          pstate->pipe_hw->cap->sblk->maxdwnscale << 16, >> +                          pipe->sspp->cap->sblk->maxdwnscale << 16, >>                             true, true); >>       if (ret) { >>           DPU_DEBUG_PLANE(pdpu, "Check plane state failed (%d)\n", ret); >> @@ -975,8 +976,8 @@ static int dpu_plane_atomic_check(struct drm_plane >> *plane, >>       min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1; >>       if (DPU_FORMAT_IS_YUV(fmt) && >> -        (!(pstate->pipe_hw->cap->features & DPU_SSPP_SCALER) || >> -         !(pstate->pipe_hw->cap->features & DPU_SSPP_CSC_ANY))) { >> +        (!(pipe->sspp->cap->features & DPU_SSPP_SCALER) || >> +         !(pipe->sspp->cap->features & DPU_SSPP_CSC_ANY))) { >>           DPU_DEBUG_PLANE(pdpu, >>                   "plane doesn't have scaler/csc for yuv\n"); >>           return -EINVAL; >> @@ -1037,12 +1038,12 @@ 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 if (pstate->pipe_hw && pstate->pipe_hw->ops.setup_csc) { >> +    else if (pstate->pipe.sspp && pstate->pipe.sspp->ops.setup_csc) { >>           const struct dpu_format *fmt = >> to_dpu_format(msm_framebuffer_format(plane->state->fb)); >>           const struct dpu_csc_cfg *csc_ptr = _dpu_plane_get_csc(pdpu, >> fmt); >>           if (csc_ptr) >> -            pstate->pipe_hw->ops.setup_csc(pstate->pipe_hw, csc_ptr); >> +            pstate->pipe.sspp->ops.setup_csc(pstate->pipe.sspp, >> csc_ptr); >>       } >>       /* flag h/w flush complete */ >> @@ -1072,6 +1073,7 @@ static void dpu_plane_sspp_atomic_update(struct >> drm_plane *plane) >>       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 = &pstate->pipe; >>       struct drm_crtc *crtc = state->crtc; >>       struct drm_framebuffer *fb = state->fb; >>       bool is_rt_pipe, update_qos_remap; >> @@ -1109,21 +1111,21 @@ static void >> dpu_plane_sspp_atomic_update(struct drm_plane *plane) >>           return; >>       } >> -    if (pstate->pipe_hw->ops.setup_rects) { >> -        pstate->pipe_hw->ops.setup_rects(pstate->pipe_hw, >> +    if (pipe->sspp->ops.setup_rects) { >> +        pipe->sspp->ops.setup_rects(pipe->sspp, >>                   &pipe_cfg, >> -                pstate->multirect_index); >> +                pipe->multirect_index); >>       } >>       _dpu_plane_setup_scaler(pdpu, pstate, fmt, false, &pipe_cfg); >> -    if (pstate->pipe_hw->ops.setup_multirect) >> -        pstate->pipe_hw->ops.setup_multirect( >> -                pstate->pipe_hw, >> -                pstate->multirect_index, >> -                pstate->multirect_mode); >> +    if (pipe->sspp->ops.setup_multirect) >> +        pipe->sspp->ops.setup_multirect( >> +                pipe->sspp, >> +                pipe->multirect_index, >> +                pipe->multirect_mode); >> -    if (pstate->pipe_hw->ops.setup_format) { >> +    if (pipe->sspp->ops.setup_format) { >>           unsigned int rotation; >>           src_flags = 0x0; >> @@ -1140,10 +1142,10 @@ static void >> dpu_plane_sspp_atomic_update(struct drm_plane *plane) >>               src_flags |= DPU_SSPP_FLIP_UD; >>           /* update format */ >> -        pstate->pipe_hw->ops.setup_format(pstate->pipe_hw, fmt, >> src_flags, >> -                pstate->multirect_index); >> +        pipe->sspp->ops.setup_format(pipe->sspp, fmt, src_flags, >> +                pipe->multirect_index); >> -        if (pstate->pipe_hw->ops.setup_cdp) { >> +        if (pipe->sspp->ops.setup_cdp) { >>               struct dpu_hw_pipe_cdp_cfg cdp_cfg; >>               memset(&cdp_cfg, 0, sizeof(struct dpu_hw_pipe_cdp_cfg)); >> @@ -1157,7 +1159,7 @@ static void dpu_plane_sspp_atomic_update(struct >> drm_plane *plane) >>                       DPU_FORMAT_IS_TILE(fmt); >>               cdp_cfg.preload_ahead = DPU_SSPP_CDP_PRELOAD_AHEAD_64; >> -            pstate->pipe_hw->ops.setup_cdp(pstate->pipe_hw, &cdp_cfg, >> pstate->multirect_index); >> +            pipe->sspp->ops.setup_cdp(pipe->sspp, &cdp_cfg, >> pipe->multirect_index); >>           } >>       } >> @@ -1191,7 +1193,7 @@ static void _dpu_plane_atomic_disable(struct >> drm_plane *plane) >>       struct dpu_plane_state *pstate = to_dpu_plane_state(state); >>       trace_dpu_plane_disable(DRMID(plane), false, >> -                pstate->multirect_mode); >> +                pstate->pipe.multirect_mode); >>       pstate->pending = true; >>   } >> @@ -1305,9 +1307,9 @@ static void dpu_plane_atomic_print_state(struct >> drm_printer *p, >>       const struct dpu_plane_state *pstate = to_dpu_plane_state(state); >>       drm_printf(p, "\tstage=%d\n", pstate->stage); >> -    drm_printf(p, "\tsspp=%s\n", pstate->pipe_hw->cap->name); >> -    drm_printf(p, "\tmultirect_mode=%s\n", >> dpu_get_multirect_mode(pstate->multirect_mode)); >> -    drm_printf(p, "\tmultirect_index=%s\n", >> dpu_get_multirect_index(pstate->multirect_index)); >> +    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)); >>   } >>   static void dpu_plane_reset(struct drm_plane *plane) >> @@ -1336,7 +1338,7 @@ static void dpu_plane_reset(struct drm_plane >> *plane) >>           return; >>       } >> -    pstate->pipe_hw = to_dpu_hw_pipe(kms->rm.sspp_blks[pdpu->pipe - >> SSPP_NONE]); >> +    pstate->pipe.sspp = to_dpu_hw_pipe(kms->rm.sspp_blks[pdpu->pipe - >> SSPP_NONE]); >>       __drm_atomic_helper_plane_reset(plane, &pstate->base); >>   } >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h >> index d745cde4ea77..b6fd6f856d6a 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h >> @@ -31,11 +31,9 @@ struct dpu_plane_state { >>       struct msm_gem_address_space *aspace; >>       enum dpu_stage stage; >>       bool needs_qos_remap; >> -    uint32_t multirect_index; >> -    uint32_t multirect_mode; >>       bool pending; >> -    struct dpu_hw_pipe *pipe_hw; >> +    struct dpu_sw_pipe pipe; > > Are you sure we dont need this to be a pointer? > > Since this is now part of plane_state, its possible that across commits, > the hw pipe can change for this plane's state. So might be better if > this is pointer in that case .... A pointer means that it is allocated and deallocated by somebody. Probably you meant that it is a pointer into the dpu_hw_pipe member (with dpu_hw_pipe having sw_pipe's for rect0 and rect1). I thought about this. And ended up with the dpu_sw_pipe having a dpu_hw_pipe pointer inside, but itself being a member of plane state. This way the plane state contains such data as multirect mode and rectangle. > >>       u64 plane_fetch_bw; >>       u64 plane_clk; -- With best wishes Dmitry From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3D095C433EF for ; Fri, 6 May 2022 21:39:53 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 12B6310EFC9; Fri, 6 May 2022 21:39:52 +0000 (UTC) Received: from mail-lj1-x230.google.com (mail-lj1-x230.google.com [IPv6:2a00:1450:4864:20::230]) by gabe.freedesktop.org (Postfix) with ESMTPS id B638410EFCC for ; Fri, 6 May 2022 21:39:50 +0000 (UTC) Received: by mail-lj1-x230.google.com with SMTP id c15so10699627ljr.9 for ; Fri, 06 May 2022 14:39:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=mjyPLoeFd41X0tFwREhvBCuwQ48xk8EujtU9e1mtlaI=; b=gFqoQvJBdfNjDyqcytFyAcXnlGpr7/lJmfI3biiOnf2jzy7/ucqH/gBhc/hnyJ65cI p01dvi72NZqS21EBGroLmif+5l99BDbeAvSLbNUnGw1QUS9jOhPWrZ8J4b3jO1lTBC9s Q6JvIJnVRBtTfeBt9RzkUy2mTNbp/obYS3CMmTsUDuBRrWviMOcjP+XeEi4ME5dkL/kd idIvToj7PSsvG41n0JiCZiOhuqiUQ0euH2/QZIR9G3dbE8Y50MHVdVrITOy19n/r008A 0mtJlksExWBK/AXwaiewGTY794idiUr8gUvP26x1edjKbx89yf8E+Lj2tEDuzdWqWpfg 48Rg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=mjyPLoeFd41X0tFwREhvBCuwQ48xk8EujtU9e1mtlaI=; b=h//3nxGvyUdIusoZe5j4MTIKigtUx47Z89N8zAr3GGUdhgjOBABfpjikTqocJjZeyG yNqUYOl50Vxbft21k+bhwcQ8Nfuw4XE0Yoi7J43+leXQFtkL/fL93ABs7BdFFm7bPd8a b6BTjdUQ+wVkqbFN3Y7T2Qm2bCNz+xm4lOwlopPtmOmHDP4B1LOF/9GBm/Qz15HE3rUL d5dCLR4VmbU05MW3vmO2jCuBYXXUeYvNLZYAN1dB8nbQBWKc+26QBnY7G/2/gIrvjN3x SchRjht6y7mo3eWA2ttBiQtMPUzWpoR+7rvGkVgRsQTeD7iYo7wfCT6HeYPJ/5yro/m3 EZ/w== X-Gm-Message-State: AOAM530Ua8cLy2i2/QDWvgD1bl1ggMoyJSURDnTjo23iuvT8md4sQbvW dknGJcPOfwkh9LlrX8fGzzyY+Q== X-Google-Smtp-Source: ABdhPJxPrngsCwhHYT+sxAbFIGDfeg+SuBoZOqu9DXj1yosZVK1L20aSj3un+HhoFcE1VWxoDiwhbA== X-Received: by 2002:a2e:150e:0:b0:24a:c41a:a3e4 with SMTP id s14-20020a2e150e000000b0024ac41aa3e4mr3149688ljd.371.1651873188855; Fri, 06 May 2022 14:39:48 -0700 (PDT) Received: from [192.168.1.211] ([37.153.55.125]) by smtp.gmail.com with ESMTPSA id c2-20020a05651200c200b0047255d211ebsm823122lfp.282.2022.05.06.14.39.48 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 06 May 2022 14:39:48 -0700 (PDT) Message-ID: <27cccd15-080e-0610-09e0-e0cfc3b85f55@linaro.org> Date: Sat, 7 May 2022 00:39:47 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.1 Subject: Re: [PATCH 10/25] drm/msm/dpu: introduce struct dpu_sw_pipe Content-Language: en-GB To: Abhinav Kumar , Bjorn Andersson , Rob Clark , Sean Paul References: <20220209172520.3719906-1-dmitry.baryshkov@linaro.org> <20220209172520.3719906-11-dmitry.baryshkov@linaro.org> <667a1dbe-5640-a92e-a9f8-a55935005083@quicinc.com> From: Dmitry Baryshkov In-Reply-To: <667a1dbe-5640-a92e-a9f8-a55935005083@quicinc.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: David Airlie , linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org, Stephen Boyd , freedreno@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 07/05/2022 00:30, Abhinav Kumar wrote: > > > On 2/9/2022 9:25 AM, Dmitry Baryshkov wrote: >> Wrap SSPP and multirect index/mode into a single structure that >> represents software view on the pipe used. >> >> Signed-off-by: Dmitry Baryshkov >> --- >>   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    |   8 +- >>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h |  16 ++- >>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 136 ++++++++++---------- >>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |   4 +- >>   4 files changed, 86 insertions(+), 78 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> index ada7d5750536..751c64012058 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> @@ -441,14 +441,14 @@ static void _dpu_crtc_blend_setup_ctl(struct >> drm_crtc *crtc) >>           pstate = to_dpu_plane_state(state); >> -        sspp_idx = pstate->pipe_hw->idx; >> +        sspp_idx = pstate->pipe.sspp->idx; >>           set_bit(sspp_idx, fetch_active); >>           stage_idx = zpos_cnt[pstate->stage]++; >>           stage_cfg.stage[pstate->stage][stage_idx] = >>                       sspp_idx; >>           stage_cfg.multirect_index[pstate->stage][stage_idx] = >> -                    pstate->multirect_index; >> +                    pstate->pipe.multirect_index; >>           /* blend config update */ >>           for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) >> @@ -1118,7 +1118,7 @@ static int dpu_crtc_atomic_check(struct drm_crtc >> *crtc, >>           pstates[cnt].dpu_pstate = to_dpu_plane_state(pstate); >>           pstates[cnt].drm_pstate = pstate; >>           pstates[cnt].stage = pstate->normalized_zpos; >> -        pstates[cnt].pipe_id = to_dpu_plane_state(pstate)->pipe_hw->idx; >> +        pstates[cnt].pipe_id = >> to_dpu_plane_state(pstate)->pipe.sspp->idx; >>           if (pipe_staged[pstates[cnt].pipe_id]) { >>               multirect_plane[multirect_count].r0 = >> @@ -1389,7 +1389,7 @@ static int _dpu_debugfs_status_show(struct >> seq_file *s, void *data) >>               state->crtc_x, state->crtc_y, state->crtc_w, >>               state->crtc_h); >>           seq_printf(s, "\tmultirect: mode: %d index: %d\n", >> -            pstate->multirect_mode, pstate->multirect_index); >> +            pstate->pipe.multirect_mode, pstate->pipe.multirect_index); >>           seq_puts(s, "\n"); >>       } >> 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 674f311f99b4..0af2bc6e5df8 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h >> @@ -159,15 +159,11 @@ struct dpu_hw_pixel_ext { >>    * @src_rect:  src ROI, caller takes into account the different >> operations >>    *             such as decimation, flip etc to program this field >>    * @dest_rect: destination ROI. >> - * @index:     index of the rectangle of SSPP >> - * @mode:      parallel or time multiplex multirect mode >>    */ >>   struct dpu_hw_pipe_cfg { >>       struct dpu_hw_fmt_layout layout; >>       struct drm_rect src_rect; >>       struct drm_rect dst_rect; >> -    enum dpu_sspp_multirect_index index; >> -    enum dpu_sspp_multirect_mode mode; >>   }; >>   /** >> @@ -218,6 +214,18 @@ struct dpu_hw_pipe_ts_cfg { >>       u64 time; >>   }; >> +/** >> + * struct dpu_sw_pipe - software pipe description >> + * @sspp:      backing SSPP pipe >> + * @index:     index of the rectangle of SSPP >> + * @mode:      parallel or time multiplex multirect mode >> + */ >> +struct dpu_sw_pipe { >> +    struct dpu_hw_pipe *sspp; >> +    enum dpu_sspp_multirect_index multirect_index; >> +    enum dpu_sspp_multirect_mode multirect_mode; >> +}; >> + >>   /** >>    * struct dpu_hw_sspp_ops - interface to the SSPP Hw driver functions >>    * Caller must call the init function to get the pipe context for >> each pipe >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> index d1f9b4bc10ac..51b5e8a3182b 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> @@ -247,7 +247,7 @@ static int _dpu_plane_calc_fill_level(struct >> drm_plane *plane, >>                   ((src_width + 32) * fmt->bpp); >>           } >>       } else { >> -        if (pstate->multirect_mode == DPU_SSPP_MULTIRECT_PARALLEL) { >> +        if (pstate->pipe.multirect_mode == >> DPU_SSPP_MULTIRECT_PARALLEL) { >>               total_fl = (fixed_buff_size / 2) * 2 / >>                   ((src_width + 32) * fmt->bpp); >>           } else { >> @@ -331,7 +331,7 @@ static void _dpu_plane_set_qos_lut(struct >> drm_plane *plane, >>               fmt ? (char *)&fmt->base.pixel_format : NULL, >>               pdpu->is_rt_pipe, total_fl, qos_lut); >> -    pstate->pipe_hw->ops.setup_creq_lut(pstate->pipe_hw, qos_lut); >> +    pstate->pipe.sspp->ops.setup_creq_lut(pstate->pipe.sspp, qos_lut); >>   } >>   /** >> @@ -383,7 +383,7 @@ static void _dpu_plane_set_danger_lut(struct >> drm_plane *plane, >>           danger_lut, >>           safe_lut); >> -    pstate->pipe_hw->ops.setup_danger_safe_lut(pstate->pipe_hw, >> +    pstate->pipe.sspp->ops.setup_danger_safe_lut(pstate->pipe.sspp, >>               danger_lut, safe_lut); >>   } >> @@ -403,9 +403,9 @@ static void _dpu_plane_set_qos_ctrl(struct >> drm_plane *plane, >>       memset(&pipe_qos_cfg, 0, sizeof(pipe_qos_cfg)); >>       if (flags & DPU_PLANE_QOS_VBLANK_CTRL) { >> -        pipe_qos_cfg.creq_vblank = >> pstate->pipe_hw->cap->sblk->creq_vblank; >> +        pipe_qos_cfg.creq_vblank = >> pstate->pipe.sspp->cap->sblk->creq_vblank; >>           pipe_qos_cfg.danger_vblank = >> -                pstate->pipe_hw->cap->sblk->danger_vblank; >> +                pstate->pipe.sspp->cap->sblk->danger_vblank; >>           pipe_qos_cfg.vblank_en = enable; >>       } >> @@ -431,7 +431,7 @@ static void _dpu_plane_set_qos_ctrl(struct >> drm_plane *plane, >>           pipe_qos_cfg.danger_vblank, >>           pdpu->is_rt_pipe); >> -    pstate->pipe_hw->ops.setup_qos_ctrl(pstate->pipe_hw, >> +    pstate->pipe.sspp->ops.setup_qos_ctrl(pstate->pipe.sspp, >>               &pipe_qos_cfg); >>   } >> @@ -450,14 +450,14 @@ static void _dpu_plane_set_ot_limit(struct >> drm_plane *plane, >>       struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); >>       memset(&ot_params, 0, sizeof(ot_params)); >> -    ot_params.xin_id = pstate->pipe_hw->cap->xin_id; >> -    ot_params.num = pstate->pipe_hw->idx - SSPP_NONE; >> +    ot_params.xin_id = pstate->pipe.sspp->cap->xin_id; >> +    ot_params.num = pstate->pipe.sspp->idx - SSPP_NONE; >>       ot_params.width = drm_rect_width(&pipe_cfg->src_rect); >>       ot_params.height = drm_rect_height(&pipe_cfg->src_rect); >>       ot_params.is_wfd = !pdpu->is_rt_pipe; >>       ot_params.frame_rate = drm_mode_vrefresh(&crtc->mode); >>       ot_params.vbif_idx = VBIF_RT; >> -    ot_params.clk_ctrl = pstate->pipe_hw->cap->clk_ctrl; >> +    ot_params.clk_ctrl = pstate->pipe.sspp->cap->clk_ctrl; >>       ot_params.rd = true; >>       dpu_vbif_set_ot_limit(dpu_kms, &ot_params); >> @@ -476,9 +476,9 @@ static void _dpu_plane_set_qos_remap(struct >> drm_plane *plane) >>       memset(&qos_params, 0, sizeof(qos_params)); >>       qos_params.vbif_idx = VBIF_RT; >> -    qos_params.clk_ctrl = pstate->pipe_hw->cap->clk_ctrl; >> -    qos_params.xin_id = pstate->pipe_hw->cap->xin_id; >> -    qos_params.num = pstate->pipe_hw->idx - SSPP_VIG0; >> +    qos_params.clk_ctrl = pstate->pipe.sspp->cap->clk_ctrl; >> +    qos_params.xin_id = pstate->pipe.sspp->cap->xin_id; >> +    qos_params.num = pstate->pipe.sspp->idx - SSPP_VIG0; >>       qos_params.is_rt = pdpu->is_rt_pipe; >>       DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d, >> clk_ctrl:%d\n", >> @@ -505,12 +505,12 @@ static void _dpu_plane_set_scanout(struct >> drm_plane *plane, >>           DPU_DEBUG_PLANE(pdpu, "not updating same src addrs\n"); >>       else if (ret) >>           DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", >> ret); >> -    else if (pstate->pipe_hw->ops.setup_sourceaddress) { >> -        trace_dpu_plane_set_scanout(pstate->pipe_hw->idx, >> +    else if (pstate->pipe.sspp->ops.setup_sourceaddress) { >> +        trace_dpu_plane_set_scanout(pstate->pipe.sspp->idx, >>                           &pipe_cfg->layout, >> -                        pstate->multirect_index); >> -        pstate->pipe_hw->ops.setup_sourceaddress(pstate->pipe_hw, >> pipe_cfg, >> -                        pstate->multirect_index); >> +                        pstate->pipe.multirect_index); >> +        pstate->pipe.sspp->ops.setup_sourceaddress(pstate->pipe.sspp, >> pipe_cfg, >> +                        pstate->pipe.multirect_index); >>       } >>   } >> @@ -553,7 +553,7 @@ static void _dpu_plane_setup_scaler3(struct >> dpu_plane *pdpu, >>               scale_cfg->src_height[i] /= chroma_subsmpl_v; >>           } >> -        if (pstate->pipe_hw->cap->features & >> +        if (pstate->pipe.sspp->cap->features & >>               BIT(DPU_SSPP_SCALER_QSEED4)) { >>               scale_cfg->preload_x[i] = DPU_QSEED4_DEFAULT_PRELOAD_H; >>               scale_cfg->preload_y[i] = DPU_QSEED4_DEFAULT_PRELOAD_V; >> @@ -624,7 +624,7 @@ static const struct dpu_csc_cfg >> *_dpu_plane_get_csc(struct dpu_plane *pdpu, cons >>       if (!DPU_FORMAT_IS_YUV(fmt)) >>           return NULL; >> -    if (BIT(DPU_SSPP_CSC_10BIT) & pstate->pipe_hw->cap->features) >> +    if (BIT(DPU_SSPP_CSC_10BIT) & pstate->pipe.sspp->cap->features) >>           csc_ptr = &dpu_csc10_YUV2RGB_601L; >>       else >>           csc_ptr = &dpu_csc_YUV2RGB_601L; >> @@ -659,8 +659,8 @@ static void _dpu_plane_setup_scaler(struct >> dpu_plane *pdpu, >>               &scaler3_cfg, &pixel_ext, fmt, >>               info->hsub, info->vsub); >> -    if (pstate->pipe_hw->ops.setup_pe) >> -        pstate->pipe_hw->ops.setup_pe(pstate->pipe_hw, >> +    if (pstate->pipe.sspp->ops.setup_pe) >> +        pstate->pipe.sspp->ops.setup_pe(pstate->pipe.sspp, >>                   &pixel_ext); >>       /** >> @@ -668,9 +668,9 @@ static void _dpu_plane_setup_scaler(struct >> dpu_plane *pdpu, >>        * bypassed. Still we need to update alpha and bitwidth >>        * ONLY for RECT0 >>        */ >> -    if (pstate->pipe_hw->ops.setup_scaler && >> -            pstate->multirect_index != DPU_SSPP_RECT_1) >> -        pstate->pipe_hw->ops.setup_scaler(pstate->pipe_hw, >> +    if (pstate->pipe.sspp->ops.setup_scaler && >> +            pstate->pipe.multirect_index != DPU_SSPP_RECT_1) >> +        pstate->pipe.sspp->ops.setup_scaler(pstate->pipe.sspp, >>                   pipe_cfg, >>                   &scaler3_cfg); >>   } >> @@ -699,10 +699,10 @@ static int _dpu_plane_color_fill(struct >> dpu_plane *pdpu, >>       fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR8888); >>       /* update sspp */ >> -    if (fmt && pstate->pipe_hw->ops.setup_solidfill) { >> -        pstate->pipe_hw->ops.setup_solidfill(pstate->pipe_hw, >> +    if (fmt && pstate->pipe.sspp->ops.setup_solidfill) { >> +        pstate->pipe.sspp->ops.setup_solidfill(pstate->pipe.sspp, >>                   (color & 0xFFFFFF) | ((alpha & 0xFF) << 24), >> -                pstate->multirect_index); >> +                pstate->pipe.multirect_index); >>           /* override scaler/decimation if solid fill */ >>           pipe_cfg.dst_rect = pstate->base.dst; >> @@ -714,15 +714,15 @@ static int _dpu_plane_color_fill(struct >> dpu_plane *pdpu, >>           pipe_cfg.src_rect.y2 = >>               drm_rect_height(&pipe_cfg.dst_rect); >> -        if (pstate->pipe_hw->ops.setup_format) >> -            pstate->pipe_hw->ops.setup_format(pstate->pipe_hw, >> +        if (pstate->pipe.sspp->ops.setup_format) >> +            pstate->pipe.sspp->ops.setup_format(pstate->pipe.sspp, >>                       fmt, DPU_SSPP_SOLID_FILL, >> -                    pstate->multirect_index); >> +                    pstate->pipe.multirect_index); >> -        if (pstate->pipe_hw->ops.setup_rects) >> -            pstate->pipe_hw->ops.setup_rects(pstate->pipe_hw, >> +        if (pstate->pipe.sspp->ops.setup_rects) >> +            pstate->pipe.sspp->ops.setup_rects(pstate->pipe.sspp, >>                       &pipe_cfg, >> -                    pstate->multirect_index); >> +                    pstate->pipe.multirect_index); >>           _dpu_plane_setup_scaler(pdpu, pstate, fmt, true, &pipe_cfg); >>       } >> @@ -734,8 +734,8 @@ void dpu_plane_clear_multirect(const struct >> drm_plane_state *drm_state) >>   { >>       struct dpu_plane_state *pstate = to_dpu_plane_state(drm_state); >> -    pstate->multirect_index = DPU_SSPP_RECT_SOLO; >> -    pstate->multirect_mode = DPU_SSPP_MULTIRECT_NONE; >> +    pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO; >> +    pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE; >>   } >>   int dpu_plane_validate_multirect_v2(struct >> dpu_multirect_plane_states *plane) >> @@ -817,8 +817,8 @@ int dpu_plane_validate_multirect_v2(struct >> dpu_multirect_plane_states *plane) >>       /* Prefer PARALLEL FETCH Mode over TIME_MX Mode */ >>       if (parallel_fetch_qualified) { >> -        pstate[R0]->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL; >> -        pstate[R1]->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL; >> +        pstate[R0]->pipe.multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL; >> +        pstate[R1]->pipe.multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL; >>           goto done; >>       } >> @@ -828,8 +828,8 @@ int dpu_plane_validate_multirect_v2(struct >> dpu_multirect_plane_states *plane) >>       if (dst[R1].y1 >= dst[R0].y2 + buffer_lines || >>           dst[R0].y1 >= dst[R1].y2 + buffer_lines) { >> -        pstate[R0]->multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX; >> -        pstate[R1]->multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX; >> +        pstate[R0]->pipe.multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX; >> +        pstate[R1]->pipe.multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX; >>       } else { >>           DPU_ERROR( >>               "No multirect mode possible for the planes (%d - %d)\n", >> @@ -839,13 +839,13 @@ int dpu_plane_validate_multirect_v2(struct >> dpu_multirect_plane_states *plane) >>       } >>   done: >> -    pstate[R0]->multirect_index = DPU_SSPP_RECT_0; >> -    pstate[R1]->multirect_index = DPU_SSPP_RECT_1; >> +    pstate[R0]->pipe.multirect_index = DPU_SSPP_RECT_0; >> +    pstate[R1]->pipe.multirect_index = DPU_SSPP_RECT_1; >>       DPU_DEBUG_PLANE(dpu_plane[R0], "R0: %d - %d\n", >> -        pstate[R0]->multirect_mode, pstate[R0]->multirect_index); >> +        pstate[R0]->pipe.multirect_mode, >> pstate[R0]->pipe.multirect_index); >>       DPU_DEBUG_PLANE(dpu_plane[R1], "R1: %d - %d\n", >> -        pstate[R1]->multirect_mode, pstate[R1]->multirect_index); >> +        pstate[R1]->pipe.multirect_mode, >> pstate[R1]->pipe.multirect_index); >>       return 0; >>   } >> @@ -937,6 +937,7 @@ 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; >>       const struct drm_crtc_state *crtc_state = NULL; >>       const struct dpu_format *fmt; >>       struct drm_rect src, dst, fb_rect = { 0 }; >> @@ -946,10 +947,10 @@ static int dpu_plane_atomic_check(struct >> drm_plane *plane, >>           crtc_state = drm_atomic_get_new_crtc_state(state, >>                                  new_plane_state->crtc); >> -    min_scale = FRAC_16_16(1, pstate->pipe_hw->cap->sblk->maxupscale); >> +    min_scale = FRAC_16_16(1, pipe->sspp->cap->sblk->maxupscale); >>       ret = drm_atomic_helper_check_plane_state(new_plane_state, >> crtc_state, >>                             min_scale, >> -                          pstate->pipe_hw->cap->sblk->maxdwnscale << 16, >> +                          pipe->sspp->cap->sblk->maxdwnscale << 16, >>                             true, true); >>       if (ret) { >>           DPU_DEBUG_PLANE(pdpu, "Check plane state failed (%d)\n", ret); >> @@ -975,8 +976,8 @@ static int dpu_plane_atomic_check(struct drm_plane >> *plane, >>       min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1; >>       if (DPU_FORMAT_IS_YUV(fmt) && >> -        (!(pstate->pipe_hw->cap->features & DPU_SSPP_SCALER) || >> -         !(pstate->pipe_hw->cap->features & DPU_SSPP_CSC_ANY))) { >> +        (!(pipe->sspp->cap->features & DPU_SSPP_SCALER) || >> +         !(pipe->sspp->cap->features & DPU_SSPP_CSC_ANY))) { >>           DPU_DEBUG_PLANE(pdpu, >>                   "plane doesn't have scaler/csc for yuv\n"); >>           return -EINVAL; >> @@ -1037,12 +1038,12 @@ 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 if (pstate->pipe_hw && pstate->pipe_hw->ops.setup_csc) { >> +    else if (pstate->pipe.sspp && pstate->pipe.sspp->ops.setup_csc) { >>           const struct dpu_format *fmt = >> to_dpu_format(msm_framebuffer_format(plane->state->fb)); >>           const struct dpu_csc_cfg *csc_ptr = _dpu_plane_get_csc(pdpu, >> fmt); >>           if (csc_ptr) >> -            pstate->pipe_hw->ops.setup_csc(pstate->pipe_hw, csc_ptr); >> +            pstate->pipe.sspp->ops.setup_csc(pstate->pipe.sspp, >> csc_ptr); >>       } >>       /* flag h/w flush complete */ >> @@ -1072,6 +1073,7 @@ static void dpu_plane_sspp_atomic_update(struct >> drm_plane *plane) >>       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 = &pstate->pipe; >>       struct drm_crtc *crtc = state->crtc; >>       struct drm_framebuffer *fb = state->fb; >>       bool is_rt_pipe, update_qos_remap; >> @@ -1109,21 +1111,21 @@ static void >> dpu_plane_sspp_atomic_update(struct drm_plane *plane) >>           return; >>       } >> -    if (pstate->pipe_hw->ops.setup_rects) { >> -        pstate->pipe_hw->ops.setup_rects(pstate->pipe_hw, >> +    if (pipe->sspp->ops.setup_rects) { >> +        pipe->sspp->ops.setup_rects(pipe->sspp, >>                   &pipe_cfg, >> -                pstate->multirect_index); >> +                pipe->multirect_index); >>       } >>       _dpu_plane_setup_scaler(pdpu, pstate, fmt, false, &pipe_cfg); >> -    if (pstate->pipe_hw->ops.setup_multirect) >> -        pstate->pipe_hw->ops.setup_multirect( >> -                pstate->pipe_hw, >> -                pstate->multirect_index, >> -                pstate->multirect_mode); >> +    if (pipe->sspp->ops.setup_multirect) >> +        pipe->sspp->ops.setup_multirect( >> +                pipe->sspp, >> +                pipe->multirect_index, >> +                pipe->multirect_mode); >> -    if (pstate->pipe_hw->ops.setup_format) { >> +    if (pipe->sspp->ops.setup_format) { >>           unsigned int rotation; >>           src_flags = 0x0; >> @@ -1140,10 +1142,10 @@ static void >> dpu_plane_sspp_atomic_update(struct drm_plane *plane) >>               src_flags |= DPU_SSPP_FLIP_UD; >>           /* update format */ >> -        pstate->pipe_hw->ops.setup_format(pstate->pipe_hw, fmt, >> src_flags, >> -                pstate->multirect_index); >> +        pipe->sspp->ops.setup_format(pipe->sspp, fmt, src_flags, >> +                pipe->multirect_index); >> -        if (pstate->pipe_hw->ops.setup_cdp) { >> +        if (pipe->sspp->ops.setup_cdp) { >>               struct dpu_hw_pipe_cdp_cfg cdp_cfg; >>               memset(&cdp_cfg, 0, sizeof(struct dpu_hw_pipe_cdp_cfg)); >> @@ -1157,7 +1159,7 @@ static void dpu_plane_sspp_atomic_update(struct >> drm_plane *plane) >>                       DPU_FORMAT_IS_TILE(fmt); >>               cdp_cfg.preload_ahead = DPU_SSPP_CDP_PRELOAD_AHEAD_64; >> -            pstate->pipe_hw->ops.setup_cdp(pstate->pipe_hw, &cdp_cfg, >> pstate->multirect_index); >> +            pipe->sspp->ops.setup_cdp(pipe->sspp, &cdp_cfg, >> pipe->multirect_index); >>           } >>       } >> @@ -1191,7 +1193,7 @@ static void _dpu_plane_atomic_disable(struct >> drm_plane *plane) >>       struct dpu_plane_state *pstate = to_dpu_plane_state(state); >>       trace_dpu_plane_disable(DRMID(plane), false, >> -                pstate->multirect_mode); >> +                pstate->pipe.multirect_mode); >>       pstate->pending = true; >>   } >> @@ -1305,9 +1307,9 @@ static void dpu_plane_atomic_print_state(struct >> drm_printer *p, >>       const struct dpu_plane_state *pstate = to_dpu_plane_state(state); >>       drm_printf(p, "\tstage=%d\n", pstate->stage); >> -    drm_printf(p, "\tsspp=%s\n", pstate->pipe_hw->cap->name); >> -    drm_printf(p, "\tmultirect_mode=%s\n", >> dpu_get_multirect_mode(pstate->multirect_mode)); >> -    drm_printf(p, "\tmultirect_index=%s\n", >> dpu_get_multirect_index(pstate->multirect_index)); >> +    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)); >>   } >>   static void dpu_plane_reset(struct drm_plane *plane) >> @@ -1336,7 +1338,7 @@ static void dpu_plane_reset(struct drm_plane >> *plane) >>           return; >>       } >> -    pstate->pipe_hw = to_dpu_hw_pipe(kms->rm.sspp_blks[pdpu->pipe - >> SSPP_NONE]); >> +    pstate->pipe.sspp = to_dpu_hw_pipe(kms->rm.sspp_blks[pdpu->pipe - >> SSPP_NONE]); >>       __drm_atomic_helper_plane_reset(plane, &pstate->base); >>   } >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h >> index d745cde4ea77..b6fd6f856d6a 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h >> @@ -31,11 +31,9 @@ struct dpu_plane_state { >>       struct msm_gem_address_space *aspace; >>       enum dpu_stage stage; >>       bool needs_qos_remap; >> -    uint32_t multirect_index; >> -    uint32_t multirect_mode; >>       bool pending; >> -    struct dpu_hw_pipe *pipe_hw; >> +    struct dpu_sw_pipe pipe; > > Are you sure we dont need this to be a pointer? > > Since this is now part of plane_state, its possible that across commits, > the hw pipe can change for this plane's state. So might be better if > this is pointer in that case .... A pointer means that it is allocated and deallocated by somebody. Probably you meant that it is a pointer into the dpu_hw_pipe member (with dpu_hw_pipe having sw_pipe's for rect0 and rect1). I thought about this. And ended up with the dpu_sw_pipe having a dpu_hw_pipe pointer inside, but itself being a member of plane state. This way the plane state contains such data as multirect mode and rectangle. > >>       u64 plane_fetch_bw; >>       u64 plane_clk; -- With best wishes Dmitry