Re: [PATCH 07/16] drm/msm/dpu: add cdm blocks to RM

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

 





On 11/30/2023 3:47 PM, Abhinav Kumar wrote:


On 8/30/2023 4:48 PM, Dmitry Baryshkov wrote:
On Thu, 31 Aug 2023 at 01:50, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote:

Add the RM APIs necessary to initialize and allocate CDM
blocks by the rest of the DPU pipeline.

... to be used by the rest?


Yes, thanks.



Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 17 +++++++++++++++++
  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h |  2 ++
  2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index f9215643c71a..7b6444a3fcb1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -8,6 +8,7 @@
  #include "dpu_kms.h"
  #include "dpu_hw_lm.h"
  #include "dpu_hw_ctl.h"
+#include "dpu_hw_cdm.h"
  #include "dpu_hw_pingpong.h"
  #include "dpu_hw_sspp.h"
  #include "dpu_hw_intf.h"
@@ -90,6 +91,9 @@ int dpu_rm_destroy(struct dpu_rm *rm)
                 }
         }

+       if (rm->cdm_blk)
+               dpu_hw_cdm_destroy(to_dpu_hw_cdm(rm->cdm_blk));
+
         for (i = 0; i < ARRAY_SIZE(rm->hw_wb); i++)
                 dpu_hw_wb_destroy(rm->hw_wb[i]);

@@ -240,6 +244,19 @@ int dpu_rm_init(struct dpu_rm *rm,
                 rm->hw_sspp[sspp->id - SSPP_NONE] = hw;
         }

+       if (cat->cdm) {
+               struct dpu_hw_cdm *hw;
+
+               hw = dpu_hw_cdm_init(cat->cdm, mmio);
+               /* CDM is optional so no need to bail out */
+               if (IS_ERR(hw)) {
+                       rc = PTR_ERR(hw);
+                       DPU_DEBUG("failed cdm object creation: err %d\n", rc);

No. If it is a part of the catalog, we should fail here as we do in other cases.


I guess, the only reason for not failing here was other hw blocks are needed even for basic display to come up but cdm is only for YUV formats.

Thats the only reason to mark this a failure which is "OK" to ignore.

But I see your point that if someone is listing this in the catalog but still RM fails thats an error.

Hence, ack.


+               } else {
+                       rm->cdm_blk = &hw->base;
+               }
+       }
+
         return 0;

  fail:
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index 2b551566cbf4..29b221491926 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -22,6 +22,7 @@ struct dpu_global_state;
   * @hw_wb: array of wb hardware resources
   * @dspp_blks: array of dspp hardware resources
   * @hw_sspp: array of sspp hardware resources
+ * @cdm_blk: cdm hardware resource
   */
  struct dpu_rm {
         struct dpu_hw_blk *pingpong_blks[PINGPONG_MAX - PINGPONG_0];
@@ -33,6 +34,7 @@ struct dpu_rm {
         struct dpu_hw_blk *merge_3d_blks[MERGE_3D_MAX - MERGE_3D_0];
         struct dpu_hw_blk *dsc_blks[DSC_MAX - DSC_0];
         struct dpu_hw_sspp *hw_sspp[SSPP_MAX - SSPP_NONE];
+       struct dpu_hw_blk *cdm_blk;

struct dpu_hw_cdm *cdm (or cdm_blk), please.

Ack.


I was going through this more. I think its better we leave this as a dpu_hw_blk because if you see the other blks in struct dpu_rm, all the blocks which are allocated dynamically / can change dynamically are of dpu_hw_blk type. That way the dpu_rm_get_assigned_resources() remains generic. Hence I would prefer to leave it this way.


  };

  /**
--
2.40.1



--
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