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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5bcbb092-1d29-f795-3be4-5ab1c708cba0@quicinc.com>
Date:   Thu, 30 Nov 2023 15:47:09 -0800
From:   Abhinav Kumar <quic_abhinavk@...cinc.com>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC:     <freedreno@...ts.freedesktop.org>, Rob Clark <robdclark@...il.com>,
        "Sean Paul" <sean@...rly.run>,
        Marijn Suijten <marijn.suijten@...ainline.org>,
        "David Airlie" <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        <dri-devel@...ts.freedesktop.org>, <quic_jesszhan@...cinc.com>,
        <quic_parellan@...cinc.com>, <quic_khsieh@...cinc.com>,
        <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 07/16] drm/msm/dpu: add cdm blocks to RM



On 8/30/2023 4:48 PM, Dmitry Baryshkov wrote:
> On Thu, 31 Aug 2023 at 01:50, Abhinav Kumar <quic_abhinavk@...cinc.com> 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@...cinc.com>
>> ---
>>   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.

> 
>>   };
>>
>>   /**
>> --
>> 2.40.1
>>
> 
> 
> --
> With best wishes
> Dmitry