Re: [PATCH 11/21] drm/msm/dpu: Add RM support for allocating CWB

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 9/5/2024 6:30 AM, Dmitry Baryshkov wrote:
On Tue, Sep 03, 2024 at 06:04:13PM GMT, Jessica Zhang wrote:


On 8/30/2024 3:16 PM, Dmitry Baryshkov wrote:
On Fri, 30 Aug 2024 at 22:28, Jessica Zhang <quic_jesszhan@xxxxxxxxxxx> wrote:



On 8/30/2024 10:18 AM, Dmitry Baryshkov wrote:
On Thu, Aug 29, 2024 at 01:48:32PM GMT, Jessica Zhang wrote:
Add support for allocating the concurrent writeback mux as part of the
WB allocation

Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>
---
    drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h |  5 ++++-
    drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c      | 30 +++++++++++++++++++++++++++--
    2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
index c17d2d356f7a..c43cb55fe1d2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -1,5 +1,7 @@
    /* SPDX-License-Identifier: GPL-2.0-only */
-/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
+/*
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
     */

    #ifndef _DPU_HW_MDSS_H
@@ -352,6 +354,7 @@ struct dpu_mdss_color {
    #define DPU_DBG_MASK_DSPP     (1 << 10)
    #define DPU_DBG_MASK_DSC      (1 << 11)
    #define DPU_DBG_MASK_CDM      (1 << 12)
+#define DPU_DBG_MASK_CWB      (1 << 13)

    /**
     * struct dpu_hw_tear_check - Struct contains parameters to configure
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index bc99b04eae3a..738e9a081b10 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -1,9 +1,10 @@
    // SPDX-License-Identifier: GPL-2.0-only
    /*
     * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
- * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
     */

+#include <drm/drm_managed.h>
    #include "msm_drv.h"
    #define pr_fmt(fmt)        "[drm:%s] " fmt, __func__
    #include "dpu_kms.h"
@@ -34,6 +35,7 @@ int dpu_rm_init(struct drm_device *dev,
               void __iomem *mmio)
    {
       int rc, i;
+    struct dpu_hw_blk_reg_map *cwb_reg_map;

       if (!rm || !cat || !mmio) {
               DPU_ERROR("invalid kms\n");
@@ -100,11 +102,35 @@ int dpu_rm_init(struct drm_device *dev,
               rm->hw_intf[intf->id - INTF_0] = hw;
       }

+    if (cat->cwb_count > 0) {
+            cwb_reg_map = drmm_kzalloc(dev,
+                            sizeof(*cwb_reg_map) * cat->cwb_count,
+                            GFP_KERNEL);

Please move CWB block pointers to dpu_rm. There is no need to allocate a
separate array.

Hi Dmitry,

Sorry, I'm not sure what you mean here. Can you clarify your comment?

This is just allocating an array of the CWB register addresses so that
the hw_wb block can use it to configure the CWB mux registers.

Excuse me. I asked to make the cwb_reg_map array a part of the
existing dpu_rm structure. This way other subblocks can access it
through dpu_rm API.

Got it, thanks for the clarification. Just wondering, is the intent here to
add CWB to rm's get_assigned_resourced?

The CWB registers will be handled by hw_wb and isn't referenced anywhere
outside of hw_wb (aside from when it's being allocated and passed into
hw_wb_init) so I'm not sure what's the benefit of adding it to the dpu_rm
struct.

To have a single point where all the blocks are handled, pretty much
like we have a single catalog where all blocks are allocated. Note how
e.g. how MERGE_3D is handled. Or how we return harware instances for
INTF or WB.

Got it, seems like you're leaning towards having CWB as a completely independent hardware block with its own dpu_hw_cwb file and struct.

FWIW, we did consider this approach at the very beginning, but decided to go with having the CWB registers configured by dpu_hw_wb under the hood because we thought it would be overkill to create a completely new struct just to program 2 registers via 1 function op.

We ended up adding the CWB mux programming to dpu_hw_wb because CWB is closely tied with WB and it mirrored how downstream code was programming CWB mux [1]

If you prefer to have CWB mux completely independent, I can switch to that instead.

[1] https://android.googlesource.com/kernel/msm-extra/display-drivers/+/e18d8e759a344ad4d86b31bbf8160cfe4c65b772/msm/sde/sde_hw_wb.c#265





Thanks,

Jessica Zhang


+
+            if (!cwb_reg_map) {
+                    DPU_ERROR("failed cwb object creation\n");
+                    return -ENOMEM;
+            }
+    }
+
+
+    for (i = 0; i < cat->cwb_count; i++) {
+            struct dpu_hw_blk_reg_map *cwb = &cwb_reg_map[i];
+
+            cwb->blk_addr = mmio + cat->cwb[i].base;
+            cwb->log_mask = DPU_DBG_MASK_CWB;
+    }
+
       for (i = 0; i < cat->wb_count; i++) {
               struct dpu_hw_wb *hw;
               const struct dpu_wb_cfg *wb = &cat->wb[i];

-            hw = dpu_hw_wb_init(dev, wb, mmio, cat->mdss_ver);
+            if (cat->cwb)
+                    hw = dpu_hw_wb_init_with_cwb(dev, wb, mmio,
+                                    cat->mdss_ver, cwb_reg_map);
+            else
+                    hw = dpu_hw_wb_init(dev, wb, mmio, cat->mdss_ver);
+
               if (IS_ERR(hw)) {
                       rc = PTR_ERR(hw);
                       DPU_ERROR("failed wb object creation: err %d\n", rc);

--
2.34.1


--
With best wishes
Dmitry



--
With best wishes
Dmitry

--
With best wishes
Dmitry



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux