[Freedreno] [PATCH] drm/msm/dp: add module parameter for PSR

2023-04-27 Thread Abhinav Kumar
On sc7280 where eDP is the primary display, PSR is causing
IGT breakage even for basic test cases like kms_atomic and
kms_atomic_transition. Most often the issue starts with below
stack so providing that as reference

Call trace:
 dpu_encoder_assign_crtc+0x64/0x6c
 dpu_crtc_enable+0x188/0x204
 drm_atomic_helper_commit_modeset_enables+0xc0/0x274
 msm_atomic_commit_tail+0x1a8/0x68c
 commit_tail+0xb0/0x160
 drm_atomic_helper_commit+0x11c/0x124
 drm_atomic_commit+0xb0/0xdc
 drm_atomic_connector_commit_dpms+0xf4/0x110
 drm_mode_obj_set_property_ioctl+0x16c/0x3b0
 drm_connector_property_set_ioctl+0x4c/0x74
 drm_ioctl_kernel+0xec/0x15c
 drm_ioctl+0x264/0x408
 __arm64_sys_ioctl+0x9c/0xd4
 invoke_syscall+0x4c/0x110
 el0_svc_common+0x94/0xfc
 do_el0_svc+0x3c/0xb0
 el0_svc+0x2c/0x7c
 el0t_64_sync_handler+0x48/0x114
 el0t_64_sync+0x190/0x194
---[ end trace  ]---
[drm-dp] dp_ctrl_push_idle: PUSH_IDLE pattern timedout

Other basic use-cases still seem to work fine hence add a
a module parameter to allow toggling psr enable/disable till
PSR related issues are hashed out with IGT.

Signed-off-by: Abhinav Kumar 
---
 drivers/gpu/drm/msm/dp/dp_display.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c
index 628b0e248db6..dba43167de66 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -28,6 +28,10 @@
 #include "dp_audio.h"
 #include "dp_debug.h"
 
+static bool psr_enabled = false;
+module_param(psr_enabled, bool, 0);
+MODULE_PARM_DESC(psr_enabled, "enable PSR for eDP and DP displays");
+
 #define HPD_STRING_SIZE 30
 
 enum {
@@ -407,7 +411,7 @@ static int dp_display_process_hpd_high(struct 
dp_display_private *dp)
 
edid = dp->panel->edid;
 
-   dp->dp_display.psr_supported = dp->panel->psr_cap.version;
+   dp->dp_display.psr_supported = dp->panel->psr_cap.version && 
psr_enabled;
 
dp->audio_supported = drm_detect_monitor_audio(edid);
dp_panel_handle_sink_request(dp->panel);
-- 
2.40.1



Re: [Freedreno] [PATCH v2 10/13] drm/msm: mdss: Add SM6375 support

2023-04-27 Thread Marijn Suijten
On 2023-04-21 00:31:19, Konrad Dybcio wrote:
> Add support for MDSS on SM6375.
> 
> Signed-off-by: Konrad Dybcio 

Reviewed-by: Marijn Suijten 

(After reusing sm6350 data, as suggested by Dmitry)

> ---
>  drivers/gpu/drm/msm/msm_mdss.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index 4e3a5f0c303c..f2470ce699f7 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -546,6 +546,15 @@ static const struct msm_mdss_data sm6350_data = {
>   .highest_bank_bit = 1,
>  };
>  
> +static const struct msm_mdss_data sm6375_data = {
> + .ubwc_version = UBWC_2_0,
> + .ubwc_dec_version = UBWC_2_0,
> + .ubwc_swizzle = 6,
> + .ubwc_static = 0x1e,
> + /* Possibly 0 for LPDDR3 */
> + .highest_bank_bit = 1,
> +};
> +
>  static const struct msm_mdss_data sm8150_data = {
>   .ubwc_version = UBWC_3_0,
>   .ubwc_dec_version = UBWC_3_0,
> @@ -580,6 +589,7 @@ static const struct of_device_id mdss_dt_match[] = {
>   { .compatible = "qcom,sc8280xp-mdss", .data = _data },
>   { .compatible = "qcom,sm6115-mdss", .data = _data },
>   { .compatible = "qcom,sm6350-mdss", .data = _data },
> + { .compatible = "qcom,sm6375-mdss", .data = _data },
>   { .compatible = "qcom,sm8150-mdss", .data = _data },
>   { .compatible = "qcom,sm8250-mdss", .data = _data },
>   { .compatible = "qcom,sm8350-mdss", .data = _data },
> 
> -- 
> 2.40.0
> 


Re: [Freedreno] [PATCH v2 08/13] drm/msm: mdss: Add SM6350 support

2023-04-27 Thread Marijn Suijten
On 2023-04-21 00:31:17, Konrad Dybcio wrote:
> Add support for MDSS on SM6350.
> 
> Signed-off-by: Konrad Dybcio 

Reviewed-by: Marijn Suijten 

> ---
>  drivers/gpu/drm/msm/msm_mdss.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index e8c93731aaa1..4e3a5f0c303c 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -538,6 +538,14 @@ static const struct msm_mdss_data sdm845_data = {
>   .highest_bank_bit = 2,
>  };
>  
> +static const struct msm_mdss_data sm6350_data = {
> + .ubwc_version = UBWC_2_0,
> + .ubwc_dec_version = UBWC_2_0,
> + .ubwc_swizzle = 6,
> + .ubwc_static = 0x1e,
> + .highest_bank_bit = 1,
> +};
> +
>  static const struct msm_mdss_data sm8150_data = {
>   .ubwc_version = UBWC_3_0,
>   .ubwc_dec_version = UBWC_3_0,
> @@ -571,6 +579,7 @@ static const struct of_device_id mdss_dt_match[] = {
>   { .compatible = "qcom,sc8180x-mdss", .data = _data },
>   { .compatible = "qcom,sc8280xp-mdss", .data = _data },
>   { .compatible = "qcom,sm6115-mdss", .data = _data },
> + { .compatible = "qcom,sm6350-mdss", .data = _data },
>   { .compatible = "qcom,sm8150-mdss", .data = _data },
>   { .compatible = "qcom,sm8250-mdss", .data = _data },
>   { .compatible = "qcom,sm8350-mdss", .data = _data },
> 
> -- 
> 2.40.0
> 


Re: [Freedreno] [PATCH] drm/msm/dpu: always program dsc active bits

2023-04-27 Thread Marijn Suijten
On 2023-04-14 16:51:52, Abhinav Kumar wrote:
> On 4/14/2023 4:11 PM, Marijn Suijten wrote:
> > On 2023-04-14 10:57:45, Abhinav Kumar wrote:
> >> On 4/14/2023 10:34 AM, Marijn Suijten wrote:
> >>> On 2023-04-14 08:48:43, Abhinav Kumar wrote:
>  On 4/14/2023 12:35 AM, Marijn Suijten wrote:
> > On 2023-04-12 10:33:15, Abhinav Kumar wrote:
> > [..]
> >>> What happens if a device boots without DSC panel connected?  Will
> >>> CTL_DSC_FLUSH be zero and not (unnecessarily, I assume) flush any of 
> >>> the
> >>> DSC blocks?  Or could this flush uninitialized state to the block?
> >>
> >> If we bootup without DSC panel connected, the kernel's cfg->dsc will be
> >> 0 and default register value of CTL_DSC_FLUSH will be 0 so it wont 
> >> flush
> >> any DSC blocks.
> >
> > Ack, that makes sense.  However, if I connect a DSC panel, then
> > disconnect it (now the register should be non-zero, but cfg->dsc will be
> > zero), and then replug a non-DSC panel multiple times, it'll get flushed
> > every time because we never clear CTL_DSC_FLUSH after that?
> 
>  If we remove it after kernel starts, that issue is there even today
>  without that change because DSI is not a hot-pluggable display so a
>  teardown wont happen when you plug out the panel. How will cfg->dsc be 0
>  then? In that case, its not a valid test as there was no indication to
>  DRM that display was disconnected so we cannot tear it down.
> >>>
> >>> The patch description itself describes hot-pluggable displays, which I
> >>> believe is the upcoming DSC support for DP?  You ask how cfg->dsc can
> >>> become zero, but this is **exactly** what the patch description
> >>> describes, and what this patch is removing the `if` for.  If we are not
> >>> allowed to discuss that scenario because it is not currently supported,
> >>> neither should we allow to apply this patch.
> >>>
> >>> With that in mind, can you re-answer the question?
> >>
> >> I didnt follow what needs to be re-answered.
> >>
> >> This patch is being sent in preparation of the DSC over DP support. This
> >> does not handle non-hotpluggable displays.
> > 
> > Good, because my question is specifically about *hotpluggable*
> > displays/panels like the upcoming DSC support for DP.  After all there
> > would be no point in me suggesting to connect and disconnect
> > non-hotpluggable displays and expect something sensible to happen,
> > wouldn't it?  Allow me to copy-paste the question again for convenience,
> > with some minor wording changes:
> > 
> > However, if I connect a DSC DP display, then disconnect it (now the
> > register should be non-zero, but cfg->dsc will be zero), and then
> > connect and reconnect a non-DSC DP display multiple times, it'll get
> > flushed every time because we never clear CTL_DSC_FLUSH after that?
> > 
> > And the missing part is: would multiple flushes be harmful in this case?
> 
> Well, you kept asking about "DSC panel" , that made me think you were 
> asking about a non-hotpluggable MIPI DSI DSC panel and not DP DSC 
> monitor. On many boards, panels can be removed/connected back to their 
> daughter card. The term "panel" confused me a bit.
> 
> Now answering your question.
> 
> Yes, it will get flushed once every hotplug thats not too bad but 
> importantly DSC wont be active as CTL_DSC_ACTIVE will be set to 0 so it 
> wont cause any issue.
> 
> 
> >> I do not think dynamic switch
> >> between DSC and non-DSC of non-hotpluggable displays needs to be
> >> discussed here as its not handled at all with or without this patch.
> >>
> >> We wanted to get early reviews on the patch. If you want this patch to
> >> be absorbed when rest of DSC over DP lands, I have no concerns with
> >> that. I wont pick this up for fixes and we will land this together with
> >> the rest of DP over DSC.
> > 
> > I don't mind when and where this lands, just want to have the semantics
> > clear around persisting the value of CTL_DSC_FLUSh in the register.
> > 
> > Regardless, this patch doesn't sound like a fix but a workaround until
> > reset_intf_cfg() is fixed to be called at the right point, and extended
> > to clear CTL_DSC_ACTIVE and flush the DSCs.  Perhaps it shouldn't have a
> > Fixes: tag for that reason, as you intend to reinstate this
> > if (cfg->dsc) condition when that is done?
> > 
> 
> Its certainly fixing the use-case of DSC to non-DSC switching. So it is 
> a fix.
> 
> But yes not the best fix possible. We have to improve it by moving this 
> to reset_intf_cfg() as I already committed to.

Ack, thanks for confirming this all and working on that, sounds good!

- Marijn


Re: [Freedreno] [PATCH v2 3/4] drm/msm/dpu: remove GC related code from dpu catalog

2023-04-27 Thread Marijn Suijten
On 2023-04-27 13:20:28, Abhinav Kumar wrote:


> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> >> @@ -127,12 +127,10 @@ enum {
> >>   /**
> >>    * DSPP sub-blocks
> >>    * @DPU_DSPP_PCC Panel color correction block
> >> - * @DPU_DSPP_GC  Gamma correction block
> >>    * @DPU_DSPP_IGC Inverse gamma correction block
> >>    */
> >>   enum {
> >>   DPU_DSPP_PCC = 0x1,
> >> -    DPU_DSPP_GC,
> >>   DPU_DSPP_IGC,
> > 
> > Don't we need to remove this one too (in the previous patch)?
> 
> Yes, we should. I thought of it right after sending this. will push a v3 
> which fixes it in the prev patch.

Yes please.  Don't forget to mention that dpu_dspp_sub_blks didn't even
have an igc member describing the block.

- Marijn

> >>   DPU_DSPP_MAX
> >>   };
> >> @@ -433,22 +431,18 @@ struct dpu_sspp_sub_blks {
> >>    * @maxwidth:   Max pixel width supported by this mixer
> >>    * @maxblendstages: Max number of blend-stages supported
> >>    * @blendstage_base:    Blend-stage register base offset
> >> - * @gc: gamma correction block
> >>    */
> >>   struct dpu_lm_sub_blks {
> >>   u32 maxwidth;
> >>   u32 maxblendstages;
> >>   u32 blendstage_base[MAX_BLOCKS];
> >> -    struct dpu_pp_blk gc;
> >>   };
> >>   /**
> >>    * struct dpu_dspp_sub_blks: Information of DSPP block
> >> - * @gc : gamma correction block
> >>    * @pcc: pixel color correction block
> >>    */
> >>   struct dpu_dspp_sub_blks {
> >> -    struct dpu_pp_blk gc;
> >>   struct dpu_pp_blk pcc;
> >>   };
> > 


Re: [Freedreno] [PATCH v2 4/4] drm/msm/dpu: drop DSPP_MSM8998_MASK from hw catalog

2023-04-27 Thread Marijn Suijten
On 2023-04-26 12:22:46, Abhinav Kumar wrote:
> Since GC and IGC masks have now been dropped DSPP_MSM8998_MASK
> is same as DSPP_SC7180_MASK. Since DSPP_SC7180_MASK is used more
> than DSPP_MSM8998_MASK, lets drop the latter.
> 
> Signed-off-by: Abhinav Kumar 

Fair enough, I'd use the oldest SoC but that'd require many more
unnecessary rename changes (or even better: we inline these flags at
some point, and drop .id fields from those already present in sblk).

Reviewed-by: Marijn Suijten 

> ---
>  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 4 ++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c  | 2 --
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> index 2b3ae84057df..5f6e4715aa04 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> @@ -127,9 +127,9 @@ static const struct dpu_pingpong_cfg msm8998_pp[] = {
>  };
>  
>  static const struct dpu_dspp_cfg msm8998_dspp[] = {
> - DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_MSM8998_MASK,
> + DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
>_dspp_sblk),
> - DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_MSM8998_MASK,
> + DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
>_dspp_sblk),
>  };
>  
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> index badfc3680485..2cabba0bb513 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -91,8 +91,6 @@
>  
>  #define MERGE_3D_SM8150_MASK (0)
>  
> -#define DSPP_MSM8998_MASK BIT(DPU_DSPP_PCC)
> -
>  #define DSPP_SC7180_MASK BIT(DPU_DSPP_PCC)
>  
>  #define INTF_SDM845_MASK (0)
> -- 
> 2.40.1
> 


Re: [Freedreno] [PATCH v2 2/4] drm/msm/dpu: remove DPU_DSPP_IGC handling in dspp flush

2023-04-27 Thread Marijn Suijten
DSPP*

On 2023-04-26 12:22:44, Abhinav Kumar wrote:
> Inverse gamma correction blocks (IGC) are not used today so lets
> remove the usage of DPU_DSPP_IGC in the dspp flush to make it easier

DSPP*

> to remove IGC from the catalog.
> 
> We can add this back when IGC is properly supported in DPU with
> one of the standard DRM properties.
> 
> Signed-off-by: Abhinav Kumar 

Reviewed-by: Marijn Suijten 

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> index 57adaebab563..b2a1f83ac72c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -330,9 +330,6 @@ static void 
> dpu_hw_ctl_update_pending_flush_dspp_sub_blocks(
>   return;
>  
>   switch (dspp_sub_blk) {
> - case DPU_DSPP_IGC:
> - ctx->pending_dspp_flush_mask[dspp - DSPP_0] |= BIT(2);
> - break;
>   case DPU_DSPP_PCC:
>   ctx->pending_dspp_flush_mask[dspp - DSPP_0] |= BIT(4);
>   break;
> -- 
> 2.40.1
> 


Re: [Freedreno] [PATCH v2 1/4] drm/msm/dpu: remove DPU_DSPP_GC handling in dspp flush

2023-04-27 Thread Marijn Suijten
On 2023-04-26 12:22:43, Abhinav Kumar wrote:
> Gamma correction blocks (GC) are not used today so lets remove
> the usage of DPU_DSPP_GC in the dspp flush to make it easier
> to remove GC from the catalog.
> 
> We can add this back when GC is properly supported in DPU with
> one of the standard DRM properties.
> 
> Signed-off-by: Abhinav Kumar 
> Reviewed-by: Dmitry Baryshkov 
> Link: 
> https://lore.kernel.org/r/20230421224721.12738-1-quic_abhin...@quicinc.com

This links to v1.  I don't think you should have this here `b4` should
add it when the definitive series is applied to a tree.

Regardless:

Reviewed-by: Marijn Suijten 

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> index bbdc95ce374a..57adaebab563 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -336,9 +336,6 @@ static void 
> dpu_hw_ctl_update_pending_flush_dspp_sub_blocks(
>   case DPU_DSPP_PCC:
>   ctx->pending_dspp_flush_mask[dspp - DSPP_0] |= BIT(4);
>   break;
> - case DPU_DSPP_GC:
> - ctx->pending_dspp_flush_mask[dspp - DSPP_0] |= BIT(5);
> - break;
>   default:
>   return;
>   }
> -- 
> 2.40.1
> 


Re: [Freedreno] [PATCH v2 3/4] drm/msm/dpu: remove GC related code from dpu catalog

2023-04-27 Thread Abhinav Kumar




On 4/27/2023 8:57 AM, Dmitry Baryshkov wrote:

On 26/04/2023 22:22, Abhinav Kumar wrote:

Since Gamma Correction (GC) block is currently unused, drop
related code from the dpu hardware catalog otherwise this
becomes a burden to carry across chipsets in the catalog.

Signed-off-by: Abhinav Kumar 
Reviewed-by: Dmitry Baryshkov 
Link: 
https://lore.kernel.org/r/20230421224721.12738-2-quic_abhin...@quicinc.com 


---
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 +---
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 6 --
  2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c

index 03f162af1a50..badfc3680485 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -91,7 +91,7 @@
  #define MERGE_3D_SM8150_MASK (0)
-#define DSPP_MSM8998_MASK BIT(DPU_DSPP_PCC) | BIT(DPU_DSPP_GC)
+#define DSPP_MSM8998_MASK BIT(DPU_DSPP_PCC)
  #define DSPP_SC7180_MASK BIT(DPU_DSPP_PCC)
@@ -449,8 +449,6 @@ static const struct dpu_lm_sub_blks 
qcm2290_lm_sblk = {

  static const struct dpu_dspp_sub_blks msm8998_dspp_sblk = {
  .pcc = {.id = DPU_DSPP_PCC, .base = 0x1700,
  .len = 0x90, .version = 0x10007},
-    .gc = { .id = DPU_DSPP_GC, .base = 0x17c0,
-    .len = 0x90, .version = 0x10007},
  };
  static const struct dpu_dspp_sub_blks sc7180_dspp_sblk = {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h

index 71584cd56fd7..e0dcef04bc61 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -127,12 +127,10 @@ enum {
  /**
   * DSPP sub-blocks
   * @DPU_DSPP_PCC Panel color correction block
- * @DPU_DSPP_GC  Gamma correction block
   * @DPU_DSPP_IGC Inverse gamma correction block
   */
  enum {
  DPU_DSPP_PCC = 0x1,
-    DPU_DSPP_GC,
  DPU_DSPP_IGC,


Don't we need to remove this one too (in the previous patch)?


Yes, we should. I thought of it right after sending this. will push a v3 
which fixes it in the prev patch.





  DPU_DSPP_MAX
  };
@@ -433,22 +431,18 @@ struct dpu_sspp_sub_blks {
   * @maxwidth:   Max pixel width supported by this mixer
   * @maxblendstages: Max number of blend-stages supported
   * @blendstage_base:    Blend-stage register base offset
- * @gc: gamma correction block
   */
  struct dpu_lm_sub_blks {
  u32 maxwidth;
  u32 maxblendstages;
  u32 blendstage_base[MAX_BLOCKS];
-    struct dpu_pp_blk gc;
  };
  /**
   * struct dpu_dspp_sub_blks: Information of DSPP block
- * @gc : gamma correction block
   * @pcc: pixel color correction block
   */
  struct dpu_dspp_sub_blks {
-    struct dpu_pp_blk gc;
  struct dpu_pp_blk pcc;
  };




[Freedreno] [PATCH v2 9/9] drm/msm: Wire up comm/cmdline override for fdinfo

2023-04-27 Thread Rob Clark
From: Rob Clark 

Also store the override strings in drm_file so that fdinfo can display
them.  We still need to keep our original copy as we could need these
override strings after the device file has been closed and drm_file
freed.

Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 24 +++-
 drivers/gpu/drm/msm/msm_drv.c   |  2 ++
 drivers/gpu/drm/msm/msm_gpu.h   | 10 ++
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index bb38e728864d..a20c2622a61f 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "adreno_gpu.h"
 #include "a6xx_gpu.h"
 #include "msm_gem.h"
@@ -398,7 +399,8 @@ int adreno_set_param(struct msm_gpu *gpu, struct 
msm_file_private *ctx,
switch (param) {
case MSM_PARAM_COMM:
case MSM_PARAM_CMDLINE: {
-   char *str, **paramp;
+   char *str, *str2, **paramp;
+   struct drm_file *file = ctx->file;
 
str = kmalloc(len + 1, GFP_KERNEL);
if (!str)
@@ -412,6 +414,13 @@ int adreno_set_param(struct msm_gpu *gpu, struct 
msm_file_private *ctx,
/* Ensure string is null terminated: */
str[len] = '\0';
 
+   /*
+* We need a 2nd copy for drm_file.. this copy can't replace
+* our internal copy in the ctx, because we may need it for
+* recovery/devcoredump after the file is already closed.
+*/
+   str2 = kstrdup(str, GFP_KERNEL);
+
mutex_lock(>lock);
 
if (param == MSM_PARAM_COMM) {
@@ -425,6 +434,19 @@ int adreno_set_param(struct msm_gpu *gpu, struct 
msm_file_private *ctx,
 
mutex_unlock(>lock);
 
+   mutex_lock(>override_lock);
+
+   if (param == MSM_PARAM_COMM) {
+   paramp = >override_comm;
+   } else {
+   paramp = >override_cmdline;
+   }
+
+   kfree(*paramp);
+   *paramp = str2;
+
+   mutex_unlock(>override_lock);
+
return 0;
}
case MSM_PARAM_SYSPROF:
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 81a1371c0307..3a74b5653e96 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -581,6 +581,7 @@ static int context_init(struct drm_device *dev, struct 
drm_file *file)
rwlock_init(>queuelock);
 
kref_init(>ref);
+   ctx->file = file;
msm_submitqueue_init(dev, ctx);
 
ctx->aspace = msm_gpu_create_private_address_space(priv->gpu, current);
@@ -603,6 +604,7 @@ static int msm_open(struct drm_device *dev, struct drm_file 
*file)
 
 static void context_close(struct msm_file_private *ctx)
 {
+   ctx->file = NULL;
msm_submitqueue_close(ctx);
msm_file_private_put(ctx);
 }
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 7a4fa1b8655b..671ce89e61b0 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -359,6 +359,16 @@ struct msm_file_private {
struct kref ref;
int seqno;
 
+   /**
+* @file: link back to the associated drm_file
+*
+* Note that msm_file_private can outlive the drm_file, ie.
+* after the drm_file is closed but before jobs submitted have
+* been cleaned up.  After the drm_file is closed this will be
+* NULL.
+*/
+   struct drm_file *file;
+
/**
 * sysprof:
 *
-- 
2.39.2



[Freedreno] [PATCH v2 8/9] drm/fdinfo: Add comm/cmdline override fields

2023-04-27 Thread Rob Clark
From: Rob Clark 

These are useful in particular for VM scenarios where the process which
has opened to drm device file is just a proxy for the real user in a VM
guest.

Signed-off-by: Rob Clark 
---
 Documentation/gpu/drm-usage-stats.rst | 18 ++
 drivers/gpu/drm/drm_file.c| 15 +++
 include/drm/drm_file.h| 19 +++
 3 files changed, 52 insertions(+)

diff --git a/Documentation/gpu/drm-usage-stats.rst 
b/Documentation/gpu/drm-usage-stats.rst
index 58dc0d3f8c58..e4877cf8089c 100644
--- a/Documentation/gpu/drm-usage-stats.rst
+++ b/Documentation/gpu/drm-usage-stats.rst
@@ -73,6 +73,24 @@ scope of each device, in which case `drm-pdev` shall be 
present as well.
 Userspace should make sure to not double account any usage statistics by using
 the above described criteria in order to associate data to individual clients.
 
+- drm-comm-override: 
+
+Returns the client executable override string.  Some drivers support letting
+userspace override this in cases where the userspace is simply a "proxy".
+Such as is the case with virglrenderer drm native context, where the host
+process is just forwarding command submission, etc, from guest userspace.
+This allows the proxy to make visible the executable name of the actual
+app in the VM guest.
+
+- drm-cmdline-override: 
+
+Returns the client cmdline override string.  Some drivers support letting
+userspace override this in cases where the userspace is simply a "proxy".
+Such as is the case with virglrenderer drm native context, where the host
+process is just forwarding command submission, etc, from guest userspace.
+This allows the proxy to make visible the cmdline of the actual app in the
+VM guest.
+
 Utilization
 ^^^
 
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 9321eb0bf020..d7514c313af1 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -178,6 +178,8 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor)
spin_lock_init(>master_lookup_lock);
mutex_init(>event_read_lock);
 
+   mutex_init(>override_lock);
+
if (drm_core_check_feature(dev, DRIVER_GEM))
drm_gem_open(dev, file);
 
@@ -292,6 +294,8 @@ void drm_file_free(struct drm_file *file)
WARN_ON(!list_empty(>event_list));
 
put_pid(file->pid);
+   kfree(file->override_comm);
+   kfree(file->override_cmdline);
kfree(file);
 }
 
@@ -995,6 +999,17 @@ void drm_show_fdinfo(struct seq_file *m, struct file *f)
   PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
}
 
+   mutex_lock(>override_lock);
+   if (file->override_comm) {
+   drm_printf(, "drm-comm-override:\t%s\n",
+  file->override_comm);
+   }
+   if (file->override_cmdline) {
+   drm_printf(, "drm-cmdline-override:\t%s\n",
+  file->override_cmdline);
+   }
+   mutex_unlock(>override_lock);
+
if (dev->driver->show_fdinfo)
dev->driver->show_fdinfo(, file);
 }
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 1339e925af52..604d05fa6f0c 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -370,6 +370,25 @@ struct drm_file {
 */
struct drm_prime_file_private prime;
 
+   /**
+* @comm: Overridden task comm
+*
+* Accessed under override_lock
+*/
+   char *override_comm;
+
+   /**
+* @cmdline: Overridden task cmdline
+*
+* Accessed under override_lock
+*/
+   char *override_cmdline;
+
+   /**
+* @override_lock: Serialize access to override_comm and 
override_cmdline
+*/
+   struct mutex override_lock;
+
/* private: */
 #if IS_ENABLED(CONFIG_DRM_LEGACY)
unsigned long lock_count; /* DRI1 legacy lock count */
-- 
2.39.2



[Freedreno] [PATCH v2 6/9] drm/msm: Add memory stats to fdinfo

2023-04-27 Thread Rob Clark
From: Rob Clark 

Use the new helper to export stats about memory usage.

v2: Drop unintended hunk
v3: Rebase

Signed-off-by: Rob Clark 
Reviewed-by: Emil Velikov 
---
 drivers/gpu/drm/msm/msm_drv.c |  2 ++
 drivers/gpu/drm/msm/msm_gem.c | 15 +++
 2 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 1e941aa77609..81a1371c0307 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -1052,6 +1052,8 @@ static void msm_show_fdinfo(struct drm_printer *p, struct 
drm_file *file)
return;
 
msm_gpu_show_fdinfo(priv->gpu, file->driver_priv, p);
+
+   drm_show_memory_stats(p, file);
 }
 
 static const struct file_operations fops = {
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index cd39b9d8abdb..20cfd86d2b32 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -1090,6 +1090,20 @@ int msm_gem_new_handle(struct drm_device *dev, struct 
drm_file *file,
return ret;
 }
 
+static enum drm_gem_object_status msm_gem_status(struct drm_gem_object *obj)
+{
+   struct msm_gem_object *msm_obj = to_msm_bo(obj);
+   enum drm_gem_object_status status = 0;
+
+   if (msm_obj->pages)
+   status |= DRM_GEM_OBJECT_RESIDENT;
+
+   if (msm_obj->madv == MSM_MADV_DONTNEED)
+   status |= DRM_GEM_OBJECT_PURGEABLE;
+
+   return status;
+}
+
 static const struct vm_operations_struct vm_ops = {
.fault = msm_gem_fault,
.open = drm_gem_vm_open,
@@ -1104,6 +1118,7 @@ static const struct drm_gem_object_funcs 
msm_gem_object_funcs = {
.vmap = msm_gem_prime_vmap,
.vunmap = msm_gem_prime_vunmap,
.mmap = msm_gem_object_mmap,
+   .status = msm_gem_status,
.vm_ops = _ops,
 };
 
-- 
2.39.2



[Freedreno] [PATCH v2 7/9] drm/doc: Relax fdinfo string constraints

2023-04-27 Thread Rob Clark
From: Rob Clark 

The restriction about no whitespace, etc, really only applies to the
usage of strings in keys.  Values can contain anything (other than
newline).

Signed-off-by: Rob Clark 
Acked-by: Tvrtko Ursulin 
---
 Documentation/gpu/drm-usage-stats.rst | 27 ++-
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/Documentation/gpu/drm-usage-stats.rst 
b/Documentation/gpu/drm-usage-stats.rst
index bfc14150452c..58dc0d3f8c58 100644
--- a/Documentation/gpu/drm-usage-stats.rst
+++ b/Documentation/gpu/drm-usage-stats.rst
@@ -24,7 +24,7 @@ File format specification
 - All keys shall be prefixed with `drm-`.
 - Whitespace between the delimiter and first non-whitespace character shall be
   ignored when parsing.
-- Neither keys or values are allowed to contain whitespace characters.
+- Keys are not allowed to contain whitespace characters.
 - Numerical key value pairs can end with optional unit string.
 - Data type of the value is fixed as defined in the specification.
 
@@ -39,12 +39,13 @@ Data types
 --
 
 -  - Unsigned integer without defining the maximum value.
--  - String excluding any above defined reserved characters or whitespace.
+-  - String excluding any above defined reserved characters or 
whitespace.
+-  - String.
 
 Mandatory fully standardised keys
 -
 
-- drm-driver: 
+- drm-driver: 
 
 String shall contain the name this driver registered as via the respective
 `struct drm_driver` data structure.
@@ -75,10 +76,10 @@ the above described criteria in order to associate data to 
individual clients.
 Utilization
 ^^^
 
-- drm-engine-:  ns
+- drm-engine-:  ns
 
 GPUs usually contain multiple execution engines. Each shall be given a stable
-and unique name (str), with possible values documented in the driver specific
+and unique name (keystr), with possible values documented in the driver 
specific
 documentation.
 
 Value shall be in specified time units which the respective GPU engine spent
@@ -90,19 +91,19 @@ larger value within a reasonable period. Upon observing a 
value lower than what
 was previously read, userspace is expected to stay with that larger previous
 value until a monotonic update is seen.
 
-- drm-engine-capacity-: 
+- drm-engine-capacity-: 
 
 Engine identifier string must be the same as the one specified in the
-drm-engine- tag and shall contain a greater than zero number in case the
+drm-engine- tag and shall contain a greater than zero number in case 
the
 exported engine corresponds to a group of identical hardware engines.
 
 In the absence of this tag parser shall assume capacity of one. Zero capacity
 is not allowed.
 
-- drm-cycles-: 
+- drm-cycles-: 
 
 Engine identifier string must be the same as the one specified in the
-drm-engine- tag and shall contain the number of busy cycles for the given
+drm-engine- tag and shall contain the number of busy cycles for the 
given
 engine.
 
 Values are not required to be constantly monotonic if it makes the driver
@@ -111,12 +112,12 @@ larger value within a reasonable period. Upon observing a 
value lower than what
 was previously read, userspace is expected to stay with that larger previous
 value until a monotonic update is seen.
 
-- drm-maxfreq-:  [Hz|MHz|KHz]
+- drm-maxfreq-:  [Hz|MHz|KHz]
 
 Engine identifier string must be the same as the one specified in the
-drm-engine- tag and shall contain the maximum frequency for the given
-engine.  Taken together with drm-cycles-, this can be used to calculate
-percentage utilization of the engine, whereas drm-engine- only reflects
+drm-engine- tag and shall contain the maximum frequency for the given
+engine.  Taken together with drm-cycles-, this can be used to calculate
+percentage utilization of the engine, whereas drm-engine- only reflects
 time active without considering what frequency the engine is operating as a
 percentage of it's maximum frequency.
 
-- 
2.39.2



[Freedreno] [PATCH v2 5/9] drm: Add fdinfo memory stats

2023-04-27 Thread Rob Clark
From: Rob Clark 

Add support to dump GEM stats to fdinfo.

v2: Fix typos, change size units to match docs, use div_u64
v3: Do it in core
v4: more kerneldoc

Signed-off-by: Rob Clark 
Reviewed-by: Emil Velikov 
Reviewed-by: Daniel Vetter 
---
 Documentation/gpu/drm-usage-stats.rst | 54 +++
 drivers/gpu/drm/drm_file.c| 99 ++-
 include/drm/drm_file.h| 19 +
 include/drm/drm_gem.h | 30 
 4 files changed, 189 insertions(+), 13 deletions(-)

diff --git a/Documentation/gpu/drm-usage-stats.rst 
b/Documentation/gpu/drm-usage-stats.rst
index 552195fb1ea3..bfc14150452c 100644
--- a/Documentation/gpu/drm-usage-stats.rst
+++ b/Documentation/gpu/drm-usage-stats.rst
@@ -52,6 +52,9 @@ String shall contain the name this driver registered as via 
the respective
 Optional fully standardised keys
 
 
+Identification
+^^
+
 - drm-pdev: 
 
 For PCI devices this should contain the PCI slot address of the device in
@@ -69,6 +72,9 @@ scope of each device, in which case `drm-pdev` shall be 
present as well.
 Userspace should make sure to not double account any usage statistics by using
 the above described criteria in order to associate data to individual clients.
 
+Utilization
+^^^
+
 - drm-engine-:  ns
 
 GPUs usually contain multiple execution engines. Each shall be given a stable
@@ -93,18 +99,6 @@ exported engine corresponds to a group of identical hardware 
engines.
 In the absence of this tag parser shall assume capacity of one. Zero capacity
 is not allowed.
 
-- drm-memory-:  [KiB|MiB]
-
-Each possible memory type which can be used to store buffer objects by the
-GPU in question shall be given a stable and unique name to be returned as the
-string here.
-
-Value shall reflect the amount of storage currently consumed by the buffer
-object belong to this client, in the respective memory region.
-
-Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB'
-indicating kibi- or mebi-bytes.
-
 - drm-cycles-: 
 
 Engine identifier string must be the same as the one specified in the
@@ -126,6 +120,42 @@ percentage utilization of the engine, whereas 
drm-engine- only reflects
 time active without considering what frequency the engine is operating as a
 percentage of it's maximum frequency.
 
+Memory
+^^
+
+- drm-memory-:  [KiB|MiB]
+
+Each possible memory type which can be used to store buffer objects by the
+GPU in question shall be given a stable and unique name to be returned as the
+string here.  The name "memory" is reserved to refer to normal system memory.
+
+Value shall reflect the amount of storage currently consumed by the buffer
+object belong to this client, in the respective memory region.
+
+Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB'
+indicating kibi- or mebi-bytes.
+
+- drm-shared-:  [KiB|MiB]
+
+The total size of buffers that are shared with another file (ie. have more
+than a single handle).
+
+- drm-private-:  [KiB|MiB]
+
+The total size of buffers that are not shared with another file.
+
+- drm-resident-:  [KiB|MiB]
+
+The total size of buffers that are resident in system memory.
+
+- drm-purgeable-:  [KiB|MiB]
+
+The total size of buffers that are purgeable.
+
+- drm-active-:  [KiB|MiB]
+
+The total size of buffers that are active on one or more rings.
+
 Implementation Details
 ==
 
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 6d5bdd684ae2..9321eb0bf020 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -42,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "drm_crtc_internal.h"
@@ -871,9 +872,105 @@ void drm_send_event(struct drm_device *dev, struct 
drm_pending_event *e)
 }
 EXPORT_SYMBOL(drm_send_event);
 
+static void print_size(struct drm_printer *p, const char *stat,
+  const char *region, size_t sz)
+{
+   const char *units[] = {"", " KiB", " MiB"};
+   unsigned u;
+
+   for (u = 0; u < ARRAY_SIZE(units) - 1; u++) {
+   if (sz < SZ_1K)
+   break;
+   sz = div_u64(sz, SZ_1K);
+   }
+
+   drm_printf(p, "drm-%s-%s:\t%zu%s\n", stat, region, sz, units[u]);
+}
+
+/**
+ * drm_print_memory_stats - A helper to print memory stats
+ * @p: The printer to print output to
+ * @stats: The collected memory stats
+ * @supported_status: Bitmask of optional stats which are available
+ * @region: The memory region
+ *
+ */
+void drm_print_memory_stats(struct drm_printer *p,
+   const struct drm_memory_stats *stats,
+   enum drm_gem_object_status supported_status,
+   const char *region)
+{
+   print_size(p, "total", region, stats->private + stats->shared);
+   print_size(p, "shared", region, stats->shared);
+   print_size(p, "active", region, stats->active);
+
+ 

[Freedreno] [PATCH v2 4/9] drm/amdgpu: Switch to fdinfo helper

2023-04-27 Thread Rob Clark
From: Rob Clark 

Signed-off-by: Rob Clark 
Reviewed-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|  3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c | 16 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h |  2 +-
 3 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index f5ffca24def4..6c0e0c614b94 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2752,7 +2752,7 @@ static const struct file_operations 
amdgpu_driver_kms_fops = {
.compat_ioctl = amdgpu_kms_compat_ioctl,
 #endif
 #ifdef CONFIG_PROC_FS
-   .show_fdinfo = amdgpu_show_fdinfo
+   .show_fdinfo = drm_show_fdinfo,
 #endif
 };
 
@@ -2807,6 +2807,7 @@ static const struct drm_driver amdgpu_kms_driver = {
.dumb_map_offset = amdgpu_mode_dumb_mmap,
.fops = _driver_kms_fops,
.release = _driver_release_kms,
+   .show_fdinfo = amdgpu_show_fdinfo,
 
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
index 99a7855ab1bc..c2fdd5e448d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
@@ -53,9 +53,8 @@ static const char *amdgpu_ip_name[AMDGPU_HW_IP_NUM] = {
[AMDGPU_HW_IP_VCN_JPEG] =   "jpeg",
 };
 
-void amdgpu_show_fdinfo(struct seq_file *m, struct file *f)
+void amdgpu_show_fdinfo(struct drm_printer *p, struct drm_file *file)
 {
-   struct drm_file *file = f->private_data;
struct amdgpu_device *adev = drm_to_adev(file->minor->dev);
struct amdgpu_fpriv *fpriv = file->driver_priv;
struct amdgpu_vm *vm = >vm;
@@ -86,18 +85,15 @@ void amdgpu_show_fdinfo(struct seq_file *m, struct file *f)
 * **
 */
 
-   seq_printf(m, "pasid:\t%u\n", fpriv->vm.pasid);
-   seq_printf(m, "drm-driver:\t%s\n", file->minor->dev->driver->name);
-   seq_printf(m, "drm-pdev:\t%04x:%02x:%02x.%d\n", domain, bus, dev, fn);
-   seq_printf(m, "drm-client-id:\t%Lu\n", vm->immediate.fence_context);
-   seq_printf(m, "drm-memory-vram:\t%llu KiB\n", vram_mem/1024UL);
-   seq_printf(m, "drm-memory-gtt: \t%llu KiB\n", gtt_mem/1024UL);
-   seq_printf(m, "drm-memory-cpu: \t%llu KiB\n", cpu_mem/1024UL);
+   drm_printf(p, "pasid:\t%u\n", fpriv->vm.pasid);
+   drm_printf(p, "drm-memory-vram:\t%llu KiB\n", vram_mem/1024UL);
+   drm_printf(p, "drm-memory-gtt: \t%llu KiB\n", gtt_mem/1024UL);
+   drm_printf(p, "drm-memory-cpu: \t%llu KiB\n", cpu_mem/1024UL);
for (hw_ip = 0; hw_ip < AMDGPU_HW_IP_NUM; ++hw_ip) {
if (!usage[hw_ip])
continue;
 
-   seq_printf(m, "drm-engine-%s:\t%Ld ns\n", amdgpu_ip_name[hw_ip],
+   drm_printf(p, "drm-engine-%s:\t%Ld ns\n", amdgpu_ip_name[hw_ip],
   ktime_to_ns(usage[hw_ip]));
}
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h
index e86834bfea1d..0398f5a159ef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h
@@ -37,6 +37,6 @@
 #include "amdgpu_ids.h"
 
 uint32_t amdgpu_get_ip_count(struct amdgpu_device *adev, int id);
-void amdgpu_show_fdinfo(struct seq_file *m, struct file *f);
+void amdgpu_show_fdinfo(struct drm_printer *p, struct drm_file *file);
 
 #endif
-- 
2.39.2



[Freedreno] [PATCH v2 2/9] drm: Add common fdinfo helper

2023-04-27 Thread Rob Clark
From: Rob Clark 

Handle a bit of the boiler-plate in a single case, and make it easier to
add some core tracked stats.  This also ensures consistent behavior
across drivers for standardised fields.

v2: Update drm-usage-stats.rst, 64b client-id, rename drm_show_fdinfo

Reviewed-by: Daniel Vetter 
Signed-off-by: Rob Clark 
---
 Documentation/gpu/drm-usage-stats.rst | 10 +++-
 drivers/gpu/drm/drm_file.c| 35 +++
 include/drm/drm_drv.h |  7 ++
 include/drm/drm_file.h|  4 +++
 4 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/Documentation/gpu/drm-usage-stats.rst 
b/Documentation/gpu/drm-usage-stats.rst
index 72d069e5dacb..552195fb1ea3 100644
--- a/Documentation/gpu/drm-usage-stats.rst
+++ b/Documentation/gpu/drm-usage-stats.rst
@@ -126,7 +126,15 @@ percentage utilization of the engine, whereas 
drm-engine- only reflects
 time active without considering what frequency the engine is operating as a
 percentage of it's maximum frequency.
 
+Implementation Details
+==
+
+Drivers should use drm_show_fdinfo() in their `struct file_operations`, and
+implement _driver.show_fdinfo if they wish to provide any stats which
+are not provided by drm_show_fdinfo().  But even driver specific stats should
+be documented above and where possible, aligned with other drivers.
+
 Driver specific implementations
-===
+---
 
 :ref:`i915-usage-stats`
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index a51ff8cee049..6d5bdd684ae2 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -148,6 +148,7 @@ bool drm_dev_needs_global_mutex(struct drm_device *dev)
  */
 struct drm_file *drm_file_alloc(struct drm_minor *minor)
 {
+   static atomic64_t ident = ATOMIC_INIT(0);
struct drm_device *dev = minor->dev;
struct drm_file *file;
int ret;
@@ -156,6 +157,8 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor)
if (!file)
return ERR_PTR(-ENOMEM);
 
+   /* Get a unique identifier for fdinfo: */
+   file->client_id = atomic64_inc_return();
file->pid = get_pid(task_pid(current));
file->minor = minor;
 
@@ -868,6 +871,38 @@ void drm_send_event(struct drm_device *dev, struct 
drm_pending_event *e)
 }
 EXPORT_SYMBOL(drm_send_event);
 
+/**
+ * drm_show_fdinfo - helper for drm file fops
+ * @seq_file: output stream
+ * @f: the device file instance
+ *
+ * Helper to implement fdinfo, for userspace to query usage stats, etc, of a
+ * process using the GPU.  See also _driver.show_fdinfo.
+ *
+ * For text output format description please see 
Documentation/gpu/drm-usage-stats.rst
+ */
+void drm_show_fdinfo(struct seq_file *m, struct file *f)
+{
+   struct drm_file *file = f->private_data;
+   struct drm_device *dev = file->minor->dev;
+   struct drm_printer p = drm_seq_file_printer(m);
+
+   drm_printf(, "drm-driver:\t%s\n", dev->driver->name);
+   drm_printf(, "drm-client-id:\t%llu\n", file->client_id);
+
+   if (dev_is_pci(dev->dev)) {
+   struct pci_dev *pdev = to_pci_dev(dev->dev);
+
+   drm_printf(, "drm-pdev:\t%04x:%02x:%02x.%d\n",
+  pci_domain_nr(pdev->bus), pdev->bus->number,
+  PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
+   }
+
+   if (dev->driver->show_fdinfo)
+   dev->driver->show_fdinfo(, file);
+}
+EXPORT_SYMBOL(drm_show_fdinfo);
+
 /**
  * mock_drm_getfile - Create a new struct file for the drm device
  * @minor: drm minor to wrap (e.g. #drm_device.primary)
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 5b86bb7603e7..5edf2a13733b 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -401,6 +401,13 @@ struct drm_driver {
   struct drm_device *dev, uint32_t handle,
   uint64_t *offset);
 
+   /**
+* @show_fdinfo:
+*
+* Print device specific fdinfo.  See 
Documentation/gpu/drm-usage-stats.rst.
+*/
+   void (*show_fdinfo)(struct drm_printer *p, struct drm_file *f);
+
/** @major: driver major number */
int major;
/** @minor: driver minor number */
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 0d1f853092ab..6de6d0e9c634 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -258,6 +258,9 @@ struct drm_file {
/** @pid: Process that opened this file. */
struct pid *pid;
 
+   /** @client_id: A unique id for fdinfo */
+   u64 client_id;
+
/** @magic: Authentication magic, see @authenticated. */
drm_magic_t magic;
 
@@ -437,6 +440,7 @@ void drm_send_event(struct drm_device *dev, struct 
drm_pending_event *e);
 void drm_send_event_timestamp_locked(struct drm_device *dev,
 struct 

[Freedreno] [PATCH v2 1/9] drm/docs: Fix usage stats typos

2023-04-27 Thread Rob Clark
From: Rob Clark 

Fix a couple missing ':'s.

Signed-off-by: Rob Clark 
Reviewed-by: Rodrigo Vivi 
---
 Documentation/gpu/drm-usage-stats.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/gpu/drm-usage-stats.rst 
b/Documentation/gpu/drm-usage-stats.rst
index b46327356e80..72d069e5dacb 100644
--- a/Documentation/gpu/drm-usage-stats.rst
+++ b/Documentation/gpu/drm-usage-stats.rst
@@ -105,7 +105,7 @@ object belong to this client, in the respective memory 
region.
 Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB'
 indicating kibi- or mebi-bytes.
 
-- drm-cycles- 
+- drm-cycles-: 
 
 Engine identifier string must be the same as the one specified in the
 drm-engine- tag and shall contain the number of busy cycles for the given
@@ -117,7 +117,7 @@ larger value within a reasonable period. Upon observing a 
value lower than what
 was previously read, userspace is expected to stay with that larger previous
 value until a monotonic update is seen.
 
-- drm-maxfreq-  [Hz|MHz|KHz]
+- drm-maxfreq-:  [Hz|MHz|KHz]
 
 Engine identifier string must be the same as the one specified in the
 drm-engine- tag and shall contain the maximum frequency for the given
-- 
2.39.2



[Freedreno] [PATCH v2 3/9] drm/msm: Switch to fdinfo helper

2023-04-27 Thread Rob Clark
From: Rob Clark 

Now that we have a common helper, use it.

Signed-off-by: Rob Clark 
Reviewed-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/msm_drv.c | 11 +--
 drivers/gpu/drm/msm/msm_gpu.c |  2 --
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 9b6f17b1261f..1e941aa77609 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -1043,23 +1043,21 @@ static const struct drm_ioctl_desc msm_ioctls[] = {
DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_QUERY, msm_ioctl_submitqueue_query, 
DRM_RENDER_ALLOW),
 };
 
-static void msm_fop_show_fdinfo(struct seq_file *m, struct file *f)
+static void msm_show_fdinfo(struct drm_printer *p, struct drm_file *file)
 {
-   struct drm_file *file = f->private_data;
struct drm_device *dev = file->minor->dev;
struct msm_drm_private *priv = dev->dev_private;
-   struct drm_printer p = drm_seq_file_printer(m);
 
if (!priv->gpu)
return;
 
-   msm_gpu_show_fdinfo(priv->gpu, file->driver_priv, );
+   msm_gpu_show_fdinfo(priv->gpu, file->driver_priv, p);
 }
 
 static const struct file_operations fops = {
.owner = THIS_MODULE,
DRM_GEM_FOPS,
-   .show_fdinfo = msm_fop_show_fdinfo,
+   .show_fdinfo = drm_show_fdinfo,
 };
 
 static const struct drm_driver msm_driver = {
@@ -1069,7 +1067,7 @@ static const struct drm_driver msm_driver = {
DRIVER_MODESET |
DRIVER_SYNCOBJ,
.open   = msm_open,
-   .postclose   = msm_postclose,
+   .postclose  = msm_postclose,
.lastclose  = drm_fb_helper_lastclose,
.dumb_create= msm_gem_dumb_create,
.dumb_map_offset= msm_gem_dumb_map_offset,
@@ -1080,6 +1078,7 @@ static const struct drm_driver msm_driver = {
 #ifdef CONFIG_DEBUG_FS
.debugfs_init   = msm_debugfs_init,
 #endif
+   .show_fdinfo= msm_show_fdinfo,
.ioctls = msm_ioctls,
.num_ioctls = ARRAY_SIZE(msm_ioctls),
.fops   = ,
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index b1647b851018..52db90e34ead 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -151,8 +151,6 @@ int msm_gpu_pm_suspend(struct msm_gpu *gpu)
 void msm_gpu_show_fdinfo(struct msm_gpu *gpu, struct msm_file_private *ctx,
 struct drm_printer *p)
 {
-   drm_printf(p, "drm-driver:\t%s\n", gpu->dev->driver->name);
-   drm_printf(p, "drm-client-id:\t%u\n", ctx->seqno);
drm_printf(p, "drm-engine-gpu:\t%llu ns\n", ctx->elapsed_ns);
drm_printf(p, "drm-cycles-gpu:\t%llu\n", ctx->cycles);
drm_printf(p, "drm-maxfreq-gpu:\t%u Hz\n", gpu->fast_rate);
-- 
2.39.2



[Freedreno] [PATCH v2 0/9] drm: fdinfo memory stats

2023-04-27 Thread Rob Clark
From: Rob Clark 

Similar motivation to other similar recent attempt[1].  But with an
attempt to have some shared code for this.  As well as documentation.

It is probably a bit UMA-centric, I guess devices with VRAM might want
some placement stats as well.  But this seems like a reasonable start.

Basic gputop support: https://patchwork.freedesktop.org/series/116236/
And already nvtop support: https://github.com/Syllo/nvtop/pull/204

I've combined the separate series to add comm/cmdline override onto
the end of this, simply out of convenience (they would otherwise
conflict in a bunch of places).

v2: Extend things to allow for multiple regions other than just system
"memory", make drm_show_memory_stats() a helper so that, drivers
can use it or not based on their needs (but in either case, re-
use drm_print_memory_stats()

[1] https://patchwork.freedesktop.org/series/112397/


Rob Clark (9):
  drm/docs: Fix usage stats typos
  drm: Add common fdinfo helper
  drm/msm: Switch to fdinfo helper
  drm/amdgpu: Switch to fdinfo helper
  drm: Add fdinfo memory stats
  drm/msm: Add memory stats to fdinfo
  drm/doc: Relax fdinfo string constraints
  drm/fdinfo: Add comm/cmdline override fields
  drm/msm: Wire up comm/cmdline override for fdinfo

 Documentation/gpu/drm-usage-stats.rst  | 109 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|   3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c |  16 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h |   2 +-
 drivers/gpu/drm/drm_file.c | 147 +
 drivers/gpu/drm/msm/adreno/adreno_gpu.c|  24 +++-
 drivers/gpu/drm/msm/msm_drv.c  |  15 ++-
 drivers/gpu/drm/msm/msm_gem.c  |  15 +++
 drivers/gpu/drm/msm/msm_gpu.c  |   2 -
 drivers/gpu/drm/msm/msm_gpu.h  |  10 ++
 include/drm/drm_drv.h  |   7 +
 include/drm/drm_file.h |  42 ++
 include/drm/drm_gem.h  |  30 +
 13 files changed, 375 insertions(+), 47 deletions(-)

-- 
2.39.2



Re: [Freedreno] [PATCH v4 18/22] drm/msm/dpu: Describe TEAR interrupt registers for DSI interfaces

2023-04-27 Thread Dmitry Baryshkov

On 27/04/2023 01:37, Marijn Suijten wrote:

All SoCs since DPU 5.0.0 have the tear interrupt registers moved out of
the PINGPONG block and into the INTF block.  Wire up the IRQ register
masks in the interrupt table for enabling, reading and clearing them.

Signed-off-by: Marijn Suijten 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 28 +++
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h |  4 
  2 files changed, 32 insertions(+)


Reviewed-by: Dmitry Baryshkov 

--
With best wishes
Dmitry



Re: [Freedreno] [PATCH v4 07/22] drm/msm/dpu: Set PINGPONG block length to zero for DPU >= 7.0.0

2023-04-27 Thread Dmitry Baryshkov

On 27/04/2023 01:37, Marijn Suijten wrote:

Despite downstream DTS stating otherwise, the PINGPONG block has no
registers starting with DPU revision 7.0.0.  TEAR registers are gone
since DPU 5.0.0 after being moved to the INTF block, and DSC registers
are gone since 7.0.0, leaving only the dither sub-block.

A future patch, part of the DSC 1.2 series, should disable DSC functions
on the PINGPONG block for all DPU >= 7.0.0 hardware.

Fixes: 4a352c2fc15a ("drm/msm/dpu: Introduce SC8280XP")
Fixes: 0e91bcbb0016 ("drm/msm/dpu: Add SM8350 to hw catalog")
Fixes: 100d7ef6995d ("drm/msm/dpu: add support for SM8450")
Signed-off-by: Marijn Suijten 
---
  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h   | 12 ++--
  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h   |  8 
  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 12 ++--
  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h   | 16 
  4 files changed, 24 insertions(+), 24 deletions(-)


https://patchwork.freedesktop.org/patch/534306/?series=116327=2

--
With best wishes
Dmitry



Re: [Freedreno] [PATCH v4 05/22] drm/msm/dpu: Fix PP_BLK_DIPHER -> DITHER typo

2023-04-27 Thread Neil Armstrong

On 27/04/2023 00:37, Marijn Suijten wrote:

SM8550 exclusively has a DITHER sub-block inside the PINGPONG block and
no other registers, hence the DITHER name of the macro and a
corresponding PINGPONG block length of zero.  However, the PP_BLK_ macro
name was typo'd to DIPHER rather than DITHER.

Fixes: efcd0107727c ("drm/msm/dpu: add support for SM8550")
Signed-off-by: Marijn Suijten 
Reviewed-by: Konrad Dybcio 
Reviewed-by: Dmitry Baryshkov 
Reviewed-by: Abhinav Kumar 
---
  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 16 
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c |  2 +-
  2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h 
b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
index 9e403034093fd..d0ab351b6a8b9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
@@ -132,28 +132,28 @@ static const struct dpu_dspp_cfg sm8550_dspp[] = {
 _dspp_sblk),
  };
  static const struct dpu_pingpong_cfg sm8550_pp[] = {
-   PP_BLK_DIPHER("pingpong_0", PINGPONG_0, 0x69000, MERGE_3D_0, 
sc7280_pp_sblk,
+   PP_BLK_DITHER("pingpong_0", PINGPONG_0, 0x69000, MERGE_3D_0, 
sc7280_pp_sblk,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
-1),
-   PP_BLK_DIPHER("pingpong_1", PINGPONG_1, 0x6a000, MERGE_3D_0, 
sc7280_pp_sblk,
+   PP_BLK_DITHER("pingpong_1", PINGPONG_1, 0x6a000, MERGE_3D_0, 
sc7280_pp_sblk,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
-1),
-   PP_BLK_DIPHER("pingpong_2", PINGPONG_2, 0x6b000, MERGE_3D_1, 
sc7280_pp_sblk,
+   PP_BLK_DITHER("pingpong_2", PINGPONG_2, 0x6b000, MERGE_3D_1, 
sc7280_pp_sblk,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
-1),
-   PP_BLK_DIPHER("pingpong_3", PINGPONG_3, 0x6c000, MERGE_3D_1, 
sc7280_pp_sblk,
+   PP_BLK_DITHER("pingpong_3", PINGPONG_3, 0x6c000, MERGE_3D_1, 
sc7280_pp_sblk,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
-1),
-   PP_BLK_DIPHER("pingpong_4", PINGPONG_4, 0x6d000, MERGE_3D_2, 
sc7280_pp_sblk,
+   PP_BLK_DITHER("pingpong_4", PINGPONG_4, 0x6d000, MERGE_3D_2, 
sc7280_pp_sblk,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
-1),
-   PP_BLK_DIPHER("pingpong_5", PINGPONG_5, 0x6e000, MERGE_3D_2, 
sc7280_pp_sblk,
+   PP_BLK_DITHER("pingpong_5", PINGPONG_5, 0x6e000, MERGE_3D_2, 
sc7280_pp_sblk,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
-1),
-   PP_BLK_DIPHER("pingpong_6", PINGPONG_6, 0x66000, MERGE_3D_3, 
sc7280_pp_sblk,
+   PP_BLK_DITHER("pingpong_6", PINGPONG_6, 0x66000, MERGE_3D_3, 
sc7280_pp_sblk,
-1,
-1),
-   PP_BLK_DIPHER("pingpong_7", PINGPONG_7, 0x66400, MERGE_3D_3, 
sc7280_pp_sblk,
+   PP_BLK_DITHER("pingpong_7", PINGPONG_7, 0x66400, MERGE_3D_3, 
sc7280_pp_sblk,
-1,
-1),
  };
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 03f162af1a50b..ca8a02debda98 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -491,7 +491,7 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
.len = 0x20, .version = 0x2},
  };
  
-#define PP_BLK_DIPHER(_name, _id, _base, _merge_3d, _sblk, _done, _rdptr) \

+#define PP_BLK_DITHER(_name, _id, _base, _merge_3d, _sblk, _done, _rdptr) \
{\
.name = _name, .id = _id, \
.base = _base, .len = 0, \



Sorry for the typo!

Reviewed-by: Neil Armstrong 


Re: [Freedreno] [PATCH v2 3/4] drm/msm/dpu: remove GC related code from dpu catalog

2023-04-27 Thread Dmitry Baryshkov

On 26/04/2023 22:22, Abhinav Kumar wrote:

Since Gamma Correction (GC) block is currently unused, drop
related code from the dpu hardware catalog otherwise this
becomes a burden to carry across chipsets in the catalog.

Signed-off-by: Abhinav Kumar 
Reviewed-by: Dmitry Baryshkov 
Link: https://lore.kernel.org/r/20230421224721.12738-2-quic_abhin...@quicinc.com
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 +---
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 6 --
  2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 03f162af1a50..badfc3680485 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -91,7 +91,7 @@
  
  #define MERGE_3D_SM8150_MASK (0)
  
-#define DSPP_MSM8998_MASK BIT(DPU_DSPP_PCC) | BIT(DPU_DSPP_GC)

+#define DSPP_MSM8998_MASK BIT(DPU_DSPP_PCC)
  
  #define DSPP_SC7180_MASK BIT(DPU_DSPP_PCC)
  
@@ -449,8 +449,6 @@ static const struct dpu_lm_sub_blks qcm2290_lm_sblk = {

  static const struct dpu_dspp_sub_blks msm8998_dspp_sblk = {
.pcc = {.id = DPU_DSPP_PCC, .base = 0x1700,
.len = 0x90, .version = 0x10007},
-   .gc = { .id = DPU_DSPP_GC, .base = 0x17c0,
-   .len = 0x90, .version = 0x10007},
  };
  
  static const struct dpu_dspp_sub_blks sc7180_dspp_sblk = {

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 71584cd56fd7..e0dcef04bc61 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -127,12 +127,10 @@ enum {
  /**
   * DSPP sub-blocks
   * @DPU_DSPP_PCC Panel color correction block
- * @DPU_DSPP_GC  Gamma correction block
   * @DPU_DSPP_IGC Inverse gamma correction block
   */
  enum {
DPU_DSPP_PCC = 0x1,
-   DPU_DSPP_GC,
DPU_DSPP_IGC,


Don't we need to remove this one too (in the previous patch)?


DPU_DSPP_MAX
  };
@@ -433,22 +431,18 @@ struct dpu_sspp_sub_blks {
   * @maxwidth:   Max pixel width supported by this mixer
   * @maxblendstages: Max number of blend-stages supported
   * @blendstage_base:Blend-stage register base offset
- * @gc: gamma correction block
   */
  struct dpu_lm_sub_blks {
u32 maxwidth;
u32 maxblendstages;
u32 blendstage_base[MAX_BLOCKS];
-   struct dpu_pp_blk gc;
  };
  
  /**

   * struct dpu_dspp_sub_blks: Information of DSPP block
- * @gc : gamma correction block
   * @pcc: pixel color correction block
   */
  struct dpu_dspp_sub_blks {
-   struct dpu_pp_blk gc;
struct dpu_pp_blk pcc;
  };
  


--
With best wishes
Dmitry



Re: [Freedreno] [PATCH v2 4/4] drm/msm/dpu: drop DSPP_MSM8998_MASK from hw catalog

2023-04-27 Thread Dmitry Baryshkov

On 26/04/2023 22:22, Abhinav Kumar wrote:

Since GC and IGC masks have now been dropped DSPP_MSM8998_MASK
is same as DSPP_SC7180_MASK. Since DSPP_SC7180_MASK is used more
than DSPP_MSM8998_MASK, lets drop the latter.

Signed-off-by: Abhinav Kumar 
---
  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 4 ++--
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c  | 2 --
  2 files changed, 2 insertions(+), 4 deletions(-)


Reviewed-by: Dmitry Baryshkov 

--
With best wishes
Dmitry



Re: [Freedreno] [PATCH v2 2/4] drm/msm/dpu: remove DPU_DSPP_IGC handling in dspp flush

2023-04-27 Thread Dmitry Baryshkov

On 26/04/2023 22:22, Abhinav Kumar wrote:

Inverse gamma correction blocks (IGC) are not used today so lets
remove the usage of DPU_DSPP_IGC in the dspp flush to make it easier
to remove IGC from the catalog.

We can add this back when IGC is properly supported in DPU with
one of the standard DRM properties.

Signed-off-by: Abhinav Kumar 


Reviewed-by: Dmitry Baryshkov 

--
With best wishes
Dmitry



Re: [Freedreno] [PATCH v10 00/10] drm/hdcp: Pull HDCP auth/exchange/check into helpers

2023-04-27 Thread Dmitry Baryshkov

On 26/04/2023 19:29, Doug Anderson wrote:

Hi,

On Wed, Apr 19, 2023 at 8:43 AM Mark Yacoub  wrote:


Hi all,
This is v10 of the HDCP patches. The patches are authored by Sean Paul.
I rebased and addressed the review comments in v6-v10.

Main change in v10 is handling the kernel test bot warnings.

Patches 1-4 focus on moving the common HDCP helpers to common DRM.
This introduces a slight change in the original intel flow
as it splits the unique driver protocol from the generic implementation.

Patches 5-7 split the HDCP flow on the i915 driver to make use of the common 
DRM helpers.

Patches 8-10 implement HDCP on MSM driver.

Thanks,
-Mark Yacoub

Sean Paul (10):
   drm/hdcp: Add drm_hdcp_atomic_check()
   drm/hdcp: Avoid changing crtc state in hdcp atomic check
   drm/hdcp: Update property value on content type and user changes
   drm/hdcp: Expand HDCP helper library for enable/disable/check
   drm/i915/hdcp: Consolidate HDCP setup/state cache
   drm/i915/hdcp: Retain hdcp_capable return codes
   drm/i915/hdcp: Use HDCP helpers for i915
   dt-bindings: msm/dp: Add bindings for HDCP registers
   arm64: dts: qcom: sc7180: Add support for HDCP in dp-controller
   drm/msm: Implement HDCP 1.x using the new drm HDCP helpers

  .../bindings/display/msm/dp-controller.yaml   |7 +-
  arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi  |8 +
  drivers/gpu/drm/display/drm_hdcp_helper.c | 1224 +
  drivers/gpu/drm/i915/display/intel_atomic.c   |8 +-
  drivers/gpu/drm/i915/display/intel_ddi.c  |   32 +-
  .../drm/i915/display/intel_display_debugfs.c  |   12 +-
  .../drm/i915/display/intel_display_types.h|   51 +-
  drivers/gpu/drm/i915/display/intel_dp_hdcp.c  |  352 ++---
  drivers/gpu/drm/i915/display/intel_dp_mst.c   |   16 +-
  drivers/gpu/drm/i915/display/intel_hdcp.c | 1060 +++---
  drivers/gpu/drm/i915/display/intel_hdcp.h |   48 +-
  drivers/gpu/drm/i915/display/intel_hdmi.c |  267 ++--
  drivers/gpu/drm/msm/Kconfig   |1 +
  drivers/gpu/drm/msm/Makefile  |1 +
  drivers/gpu/drm/msm/dp/dp_catalog.c   |  156 +++
  drivers/gpu/drm/msm/dp/dp_catalog.h   |   18 +
  drivers/gpu/drm/msm/dp/dp_debug.c |   46 +-
  drivers/gpu/drm/msm/dp/dp_debug.h |   11 +-
  drivers/gpu/drm/msm/dp/dp_display.c   |   39 +-
  drivers/gpu/drm/msm/dp/dp_display.h   |5 +
  drivers/gpu/drm/msm/dp/dp_drm.c   |   39 +-
  drivers/gpu/drm/msm/dp/dp_drm.h   |7 +
  drivers/gpu/drm/msm/dp/dp_hdcp.c  |  389 ++
  drivers/gpu/drm/msm/dp/dp_hdcp.h  |   33 +
  drivers/gpu/drm/msm/dp/dp_parser.c|   14 +
  drivers/gpu/drm/msm/dp/dp_parser.h|4 +
  drivers/gpu/drm/msm/dp/dp_reg.h   |   30 +-
  drivers/gpu/drm/msm/msm_atomic.c  |   19 +
  include/drm/display/drm_hdcp.h|  296 
  include/drm/display/drm_hdcp_helper.h |   23 +
  30 files changed, 2867 insertions(+), 1349 deletions(-)


Mark asked me if I had any advice for getting this patch series
landed. I haven't been following the patch series super closely, but
as I understand it:

1. The first several patches (the generic ones) seem fairly well
reviewed and haven't changed in any significant ways in a while. The
ideal place to land these would be drm-misc, I think.

2. The i915 patches also seem OK to land. The ideal place would be the
Intel DRM tree, I think.

3. The msm patches are not fully baked yet. Not only is there a kernel
bot complaint on patch #10, but Mark also said that comments from v6
haven't yet fully been addressed and he's talked with Dmitry on IRC
about this and has a plan to move forward.


The question becomes: can/should we land the generic and maybe the
i915 patches now while the msm patches are reworked. Do folks have an
opinion here? If we're OK landing some of the patches, I guess we have
a few options:

a) Just land the generic patches to drm-misc and put the i915 ones on
the backburner until drm-misc has made it to somewhere that the
drm-intel tree is based on. If we want to go this route and nobody
objects, I don't mind being the person who does the gruntwork of
actually landing them on drm-misc-next, though I certainly wouldn't
rush to make sure that nobody is unhappy with this idea.

b) Land the generic patches in some type of immutable branch so they
can be pulled into drm-misc and the Intel DRM tree. Someone more
senior to me would need to help with this, but if we really want to go
this way I can poke folks on IRC.

c) Land the generic and Intel patches in the Intel tree. The msm
patches would not be able to land until these trickled up the chain,
but the msm patches aren't fully ready yet anyway so maybe this is OK.

d) Land the generic and Intel patches in the drm-misc tree. If folks
are OK with this I can be the person to pull the trigger, but I'd want
to be very sure that Intel DRM 

Re: [Freedreno] [PATCH v2 07/13] drm/msm/dpu: Add SM6350 support

2023-04-27 Thread Marijn Suijten
On 2023-04-27 17:37:42, Marijn Suijten wrote:
> On 2023-04-21 00:31:16, Konrad Dybcio wrote:
> > Add SM6350 support to the DPU1 driver to enable display output.
> > 
> > Signed-off-by: Konrad Dybcio 
> > Signed-off-by: Konrad Dybcio 
> 
> After addressing the comments from Dmitry (CURSOR0->DMA1 and
> CURSOR1->DMA2), this is:
> 
> Reviewed-by: Marijn Suijten 
> 
> See below for some nits.

Actually found one glaring issue that might explain why INTF TE wasn't
working for you the other day!

> > ---
> >  .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_4_sm6350.h | 191 
> > +
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c |   1 +
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h |   3 +
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c|   1 +
> >  4 files changed, 196 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_4_sm6350.h 
> > b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_4_sm6350.h
> > new file mode 100644
> > index ..687a508cbaa6
> > --- /dev/null
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_4_sm6350.h
> > @@ -0,0 +1,191 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights 
> > reserved.
> > + * Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved.
> > + * Copyright (c) 2023, Linaro Limited
> > + */
> > +
> > +#ifndef _DPU_6_4_SM6350_H
> > +#define _DPU_6_4_SM6350_H
> > +
> > +static const struct dpu_caps sm6350_dpu_caps = {
> > +   .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
> > +   .max_mixer_blendstages = 0x7,
> > +   .qseed_type = DPU_SSPP_SCALER_QSEED4,
> 
> I thought it was QSEED3LITE, but doesn't really matter as both are
> handled similarly.  It'll anyway change when I resubmit:
> 
> https://lore.kernel.org/linux-arm-msm/20230215-sspp-scaler-version-v1-0-416b1500b...@somainline.org/T/#u
> 
> which should hardcode the register value directly, making this field
> superfluous.
> 
> > +   .has_src_split = true,
> > +   .has_dim_layer = true,
> > +   .has_idle_pc = true,
> > +   .max_linewidth = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
> > +   .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
> > +};
> > +
> > +static const struct dpu_ubwc_cfg sm6350_ubwc_cfg = {
> > +   .ubwc_version = DPU_HW_UBWC_VER_20,
> > +   .ubwc_swizzle = 6,
> > +   .highest_bank_bit = 1,
> > +};
> > +
> > +static const struct dpu_mdp_cfg sm6350_mdp[] = {
> > +   {
> > +   .name = "top_0", .id = MDP_TOP,
> > +   .base = 0x0, .len = 0x494,
> > +   .features = 0,
> > +   .clk_ctrls[DPU_CLK_CTRL_VIG0] = { .reg_off = 0x2ac, .bit_off = 0 },
> > +   .clk_ctrls[DPU_CLK_CTRL_DMA0] = { .reg_off = 0x2ac, .bit_off = 8 },
> > +   .clk_ctrls[DPU_CLK_CTRL_DMA1] = { .reg_off = 0x2b4, .bit_off = 8 },
> > +   .clk_ctrls[DPU_CLK_CTRL_DMA2] = { .reg_off = 0x2c4, .bit_off = 8 },
> > +   .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = { .reg_off = 0x2bc, .bit_off = 20 },
> > +   },
> > +};
> > +
> > +static const struct dpu_ctl_cfg sm6350_ctl[] = {
> > +   {
> > +   .name = "ctl_0", .id = CTL_0,
> > +   .base = 0x1000, .len = 0x1dc,
> > +   .features = BIT(DPU_CTL_ACTIVE_CFG),
> > +   .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 9),
> > +   },
> > +   {
> > +   .name = "ctl_1", .id = CTL_1,
> > +   .base = 0x1200, .len = 0x1dc,
> > +   .features = BIT(DPU_CTL_ACTIVE_CFG),
> > +   .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 10),
> > +   },
> > +   {
> > +   .name = "ctl_2", .id = CTL_2,
> > +   .base = 0x1400, .len = 0x1dc,
> > +   .features = BIT(DPU_CTL_ACTIVE_CFG),
> > +   .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 11),
> > +   },
> > +   {
> > +   .name = "ctl_3", .id = CTL_3,
> > +   .base = 0x1600, .len = 0x1dc,
> > +   .features = BIT(DPU_CTL_ACTIVE_CFG),
> > +   .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 12),
> > +   },
> > +};
> > +
> > +static const struct dpu_sspp_cfg sm6350_sspp[] = {
> > +   SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, 0x1f8, VIG_SC7180_MASK,
> > +sc7180_vig_sblk_0, 0,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
> > +   SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, 0x1f8, DMA_SDM845_MASK,
> > +sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> > +   SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, 0x1f8, DMA_CURSOR_SDM845_MASK,
> > +sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> > +   SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, 0x1f8, DMA_CURSOR_SDM845_MASK,
> > +sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> > +};
> > +
> > +static const struct dpu_lm_cfg sm6350_lm[] = {
> > +   LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK,
> > +   _lm_sblk, PINGPONG_0, LM_1, DSPP_0),
> > +   LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK,
> > +   _lm_sblk, PINGPONG_1, LM_0, 0),
> 
> These two entries are indented with two tabs and have one character too
> many to align with the opening parenthesis on the previous line.  Can we
> please settle on a single style, as this commit mostly uses 

Re: [Freedreno] [PATCH v2 09/13] drm/msm/dpu: Add SM6375 support

2023-04-27 Thread Marijn Suijten
On 2023-04-21 00:31:18, Konrad Dybcio wrote:
> Add basic SM6375 support to the DPU1 driver to enable display output.
> 
> Signed-off-by: Konrad Dybcio 
> ---
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_4_sm6350.h |   5 -
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_9_sm6375.h | 152 
> +
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c |  14 ++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h |   1 +
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c|   1 +
>  5 files changed, 168 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_4_sm6350.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_4_sm6350.h
> index 687a508cbaa6..d46b43964be6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_4_sm6350.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_4_sm6350.h
> @@ -126,11 +126,6 @@ static const struct dpu_vbif_cfg sm6350_vbif[] = {
>   },
>  };
>  
> -static const struct dpu_qos_lut_entry sm6350_qos_linear_macrotile[] = {
> - {.fl = 0, .lut = 0x0011223344556677 },
> - {.fl = 0, .lut = 0x0011223445566777 },
> -};
> -
>  static const struct dpu_perf_cfg sm6350_perf_data = {
>   .max_bw_low = 420,
>   .max_bw_high = 510,
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_9_sm6375.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_9_sm6375.h
> new file mode 100644
> index ..19ca0051e072
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_9_sm6375.h
> @@ -0,0 +1,152 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved.
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +#ifndef _DPU_6_9_SM6375_H
> +#define _DPU_6_9_SM6375_H
> +
> +static const struct dpu_caps sm6375_dpu_caps = {
> + .max_mixer_width = 2048,
> + .max_mixer_blendstages = 0x4,
> + .qseed_type = DPU_SSPP_SCALER_QSEED4,
> + .has_dim_layer = true,
> + .has_idle_pc = true,
> + .max_linewidth = 2160,
> + .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
> +};
> +
> +static const struct dpu_ubwc_cfg sm6375_ubwc_cfg = {
> + .ubwc_version = DPU_HW_UBWC_VER_20,
> + .ubwc_swizzle = 6,
> + .highest_bank_bit = 1,
> +};
> +
> +static const struct dpu_mdp_cfg sm6375_mdp[] = {
> + {
> + .name = "top_0", .id = MDP_TOP,
> + .base = 0x0, .len = 0x494,
> + .features = 0,
> + .clk_ctrls[DPU_CLK_CTRL_VIG0] = { .reg_off = 0x2ac, .bit_off = 0 },
> + .clk_ctrls[DPU_CLK_CTRL_DMA0] = { .reg_off = 0x2ac, .bit_off = 8 },
> + },
> +};
> +
> +static const struct dpu_ctl_cfg sm6375_ctl[] = {
> + {
> + .name = "ctl_0", .id = CTL_0,
> + .base = 0x1000, .len = 0x1dc,
> + .features = BIT(DPU_CTL_ACTIVE_CFG),
> + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 9),
> + },
> +};
> +
> +static const struct dpu_sspp_cfg sm6375_sspp[] = {
> + SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, 0x1f8, VIG_SC7180_MASK,
> +  sm6115_vig_sblk_0, 0, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
> + SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, 0x1f8, DMA_SDM845_MASK,
> +  sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> +};
> +
> +static const struct dpu_lm_cfg sm6375_lm[] = {
> + LM_BLK("lm_0", LM_0, 0x44000, MIXER_QCM2290_MASK,
> + _lm_sblk, PINGPONG_0, 0, DSPP_0),

Same indentation nit here as in SM6350.

> +};
> +
> +static const struct dpu_dspp_cfg sm6375_dspp[] = {
> + DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
> +  _dspp_sblk),
> +};
> +
> +static const struct dpu_pingpong_cfg sm6375_pp[] = {
> + PP_BLK("pingpong_0", PINGPONG_0, 0x7, PINGPONG_SM8150_MASK, 0, 
> sdm845_pp_sblk,
> +DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> +-1),
> +};
> +
> +static const struct dpu_intf_cfg sm6375_intf[] = {
> + INTF_BLK("intf_0", INTF_0, 0x0, 0x2c0, INTF_NONE, 0, 0, 0, 0, 0),
> + INTF_BLK_DSI_TE("intf_1", INTF_1, 0x6a800, 0x2c0, INTF_DSI, 0, 24, 
> INTF_SC7280_MASK,
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 26),
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 27),
> + DPU_IRQ_IDX(MDP_INTF1_TEAR_INTR, 2)),
> +};
> +
> +static const struct dpu_vbif_cfg sm6375_vbif[] = {
> + {
> + .name = "vbif_0", .id = VBIF_RT,
> + .base = 0, .len = 0x2008,
> + .features = BIT(DPU_VBIF_QOS_REMAP),
> + .xin_halt_timeout = 0x4000,
> + .qos_rp_remap_size = 0x40,
> + .qos_rt_tbl = {
> + .npriority_lvl = ARRAY_SIZE(sdm845_rt_pri_lvl),
> + .priority_lvl = sdm845_rt_pri_lvl,
> + },
> + .qos_nrt_tbl = {
> + .npriority_lvl = ARRAY_SIZE(sdm845_nrt_pri_lvl),
> + .priority_lvl = sdm845_nrt_pri_lvl,
> + },
> + .memtype_count = 14,
> + .memtype = {3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3},
> + },
> +};
> +
> 

Re: [Freedreno] [PATCH v2 1/3] drm: Create support for Write-Only property blob

2023-04-27 Thread Sean Paul
On Thu, Apr 27, 2023 at 5:59 AM Daniel Vetter  wrote:
>
> On Fri, Apr 21, 2023 at 12:27:47PM -0400, Mark Yacoub wrote:
> > From: Mark Yacoub 
> >
> > [Why]
> > User space might need to inject data into the kernel without allowing it
> > to be read again by any user space.
> > An example of where this is particularly useful is secret keys fetched
> > by user space and injected into the kernel to enable content protection.
> >
> > [How]
> > Create a DRM_MODE_CREATE_BLOB_WRITE_ONLY flag used by user space to
> > create a blob and mark the blob as write only.
> > On reading back the blob, data will be not be copied if it's a write
> > only blob
>
> This makes no sense at all, why would you want to disallow reading?
> Userspace already knows the key, there's not much point in hiding it from
> userspace?

There are varying levels of trust amongst userspace applications. For
example, in CrOS we trust portions of Chrome to handle the key
securely, but would like to avoid access from other portions, or users
from exposing the key via modetest output. We could play whack-a-mole
and try to patch up all untrusted userspace, but that doesn't seem
like a scalable solution.

Sean

>
> Also for new uapi we need the igt patches and userspace, please link
> those.
> -Daniel
>
> >
> > Signed-off-by: Mark Yacoub 
> > ---
> >  drivers/gpu/drm/drm_property.c | 3 ++-
> >  include/drm/drm_property.h | 2 ++
> >  include/uapi/drm/drm_mode.h| 6 ++
> >  3 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
> > index dfec479830e49..afedf7109d002 100644
> > --- a/drivers/gpu/drm/drm_property.c
> > +++ b/drivers/gpu/drm/drm_property.c
> > @@ -765,7 +765,7 @@ int drm_mode_getblob_ioctl(struct drm_device *dev,
> >   if (!blob)
> >   return -ENOENT;
> >
> > - if (out_resp->length == blob->length) {
> > + if (out_resp->length == blob->length && !blob->is_write_only) {
> >   if (copy_to_user(u64_to_user_ptr(out_resp->data),
> >blob->data,
> >blob->length)) {
> > @@ -800,6 +800,7 @@ int drm_mode_createblob_ioctl(struct drm_device *dev,
> >   ret = -EFAULT;
> >   goto out_blob;
> >   }
> > + blob->is_write_only = out_resp->flags & 
> > DRM_MODE_CREATE_BLOB_WRITE_ONLY;
> >
> >   /* Dropping the lock between create_blob and our access here is safe
> >* as only the same file_priv can remove the blob; at this point, it 
> > is
> > diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
> > index 65bc9710a4702..700782f021b99 100644
> > --- a/include/drm/drm_property.h
> > +++ b/include/drm/drm_property.h
> > @@ -205,6 +205,7 @@ struct drm_property {
> >   *   _mode_config.property_blob_list.
> >   * @head_file: entry on the per-file blob list in _file.blobs list.
> >   * @length: size of the blob in bytes, invariant over the lifetime of the 
> > object
> > + * @is_write_only: user space can't read the blob data.
> >   * @data: actual data, embedded at the end of this structure
> >   *
> >   * Blobs are used to store bigger values than what fits directly into the 
> > 64
> > @@ -219,6 +220,7 @@ struct drm_property_blob {
> >   struct list_head head_global;
> >   struct list_head head_file;
> >   size_t length;
> > + bool is_write_only;
> >   void *data;
> >  };
> >
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 46becedf5b2fc..10403c9a73082 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -1168,6 +1168,9 @@ struct drm_format_modifier {
> >   __u64 modifier;
> >  };
> >
> > +#define DRM_MODE_CREATE_BLOB_WRITE_ONLY
> > \
> > + (1 << 0) /* data of the blob can't be read by user space */
> > +
> >  /**
> >   * struct drm_mode_create_blob - Create New blob property
> >   *
> > @@ -1181,6 +1184,9 @@ struct drm_mode_create_blob {
> >   __u32 length;
> >   /** @blob_id: Return: new property ID. */
> >   __u32 blob_id;
> > + /** Flags for special handling. */
> > + __u32 flags;
> > + __u32 pad;
> >  };
> >
> >  /**
> > --
> > 2.40.0.634.g4ca3ef3211-goog
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch


Re: [Freedreno] [PATCH v2 07/13] drm/msm/dpu: Add SM6350 support

2023-04-27 Thread Marijn Suijten
On 2023-04-21 00:31:16, Konrad Dybcio wrote:
> Add SM6350 support to the DPU1 driver to enable display output.
> 
> Signed-off-by: Konrad Dybcio 
> Signed-off-by: Konrad Dybcio 

After addressing the comments from Dmitry (CURSOR0->DMA1 and
CURSOR1->DMA2), this is:

Reviewed-by: Marijn Suijten 

See below for some nits.

> ---
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_4_sm6350.h | 191 
> +
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c |   1 +
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h |   3 +
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c|   1 +
>  4 files changed, 196 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_4_sm6350.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_4_sm6350.h
> new file mode 100644
> index ..687a508cbaa6
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_4_sm6350.h
> @@ -0,0 +1,191 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved.
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +#ifndef _DPU_6_4_SM6350_H
> +#define _DPU_6_4_SM6350_H
> +
> +static const struct dpu_caps sm6350_dpu_caps = {
> + .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
> + .max_mixer_blendstages = 0x7,
> + .qseed_type = DPU_SSPP_SCALER_QSEED4,

I thought it was QSEED3LITE, but doesn't really matter as both are
handled similarly.  It'll anyway change when I resubmit:

https://lore.kernel.org/linux-arm-msm/20230215-sspp-scaler-version-v1-0-416b1500b...@somainline.org/T/#u

which should hardcode the register value directly, making this field
superfluous.

> + .has_src_split = true,
> + .has_dim_layer = true,
> + .has_idle_pc = true,
> + .max_linewidth = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
> + .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
> +};
> +
> +static const struct dpu_ubwc_cfg sm6350_ubwc_cfg = {
> + .ubwc_version = DPU_HW_UBWC_VER_20,
> + .ubwc_swizzle = 6,
> + .highest_bank_bit = 1,
> +};
> +
> +static const struct dpu_mdp_cfg sm6350_mdp[] = {
> + {
> + .name = "top_0", .id = MDP_TOP,
> + .base = 0x0, .len = 0x494,
> + .features = 0,
> + .clk_ctrls[DPU_CLK_CTRL_VIG0] = { .reg_off = 0x2ac, .bit_off = 0 },
> + .clk_ctrls[DPU_CLK_CTRL_DMA0] = { .reg_off = 0x2ac, .bit_off = 8 },
> + .clk_ctrls[DPU_CLK_CTRL_DMA1] = { .reg_off = 0x2b4, .bit_off = 8 },
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = { .reg_off = 0x2c4, .bit_off = 8 },
> + .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = { .reg_off = 0x2bc, .bit_off = 20 },
> + },
> +};
> +
> +static const struct dpu_ctl_cfg sm6350_ctl[] = {
> + {
> + .name = "ctl_0", .id = CTL_0,
> + .base = 0x1000, .len = 0x1dc,
> + .features = BIT(DPU_CTL_ACTIVE_CFG),
> + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 9),
> + },
> + {
> + .name = "ctl_1", .id = CTL_1,
> + .base = 0x1200, .len = 0x1dc,
> + .features = BIT(DPU_CTL_ACTIVE_CFG),
> + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 10),
> + },
> + {
> + .name = "ctl_2", .id = CTL_2,
> + .base = 0x1400, .len = 0x1dc,
> + .features = BIT(DPU_CTL_ACTIVE_CFG),
> + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 11),
> + },
> + {
> + .name = "ctl_3", .id = CTL_3,
> + .base = 0x1600, .len = 0x1dc,
> + .features = BIT(DPU_CTL_ACTIVE_CFG),
> + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 12),
> + },
> +};
> +
> +static const struct dpu_sspp_cfg sm6350_sspp[] = {
> + SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, 0x1f8, VIG_SC7180_MASK,
> +  sc7180_vig_sblk_0, 0,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
> + SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, 0x1f8, DMA_SDM845_MASK,
> +  sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> + SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, 0x1f8, DMA_CURSOR_SDM845_MASK,
> +  sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, 0x1f8, DMA_CURSOR_SDM845_MASK,
> +  sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> +};
> +
> +static const struct dpu_lm_cfg sm6350_lm[] = {
> + LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK,
> + _lm_sblk, PINGPONG_0, LM_1, DSPP_0),
> + LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK,
> + _lm_sblk, PINGPONG_1, LM_0, 0),

These two entries are indented with two tabs and have one character too
many to align with the opening parenthesis on the previous line.  Can we
please settle on a single style, as this commit mostly uses tabs+spaces
to align with the opening parenthesis?

Dmitry vouched for `cino=(0` (when in unclosed parenthesis, align next
line with zero extra characters to the opening parenthesis), but I find
double tabs more convenient as it doesn't require reindenting when
changing the name of the 

Re: [Freedreno] [RFC 2/3] drm/msm: Rework get_comm_cmdline() helper

2023-04-27 Thread Rob Clark
On Thu, Apr 27, 2023 at 2:39 AM Daniel Vetter  wrote:
>
> On Fri, Apr 21, 2023 at 07:47:26AM -0700, Rob Clark wrote:
> > On Fri, Apr 21, 2023 at 2:33 AM Emil Velikov  
> > wrote:
> > >
> > > Greeting all,
> > >
> > > Sorry for the delay - Easter Holidays, food coma and all that :-)
> > >
> > > On Tue, 18 Apr 2023 at 15:31, Rob Clark  wrote:
> > > >
> > > > On Tue, Apr 18, 2023 at 1:34 AM Daniel Vetter  wrote:
> > > > >
> > > > > On Tue, Apr 18, 2023 at 09:27:49AM +0100, Tvrtko Ursulin wrote:
> > > > > >
> > > > > > On 17/04/2023 21:12, Rob Clark wrote:
> > > > > > > From: Rob Clark 
> > > > > > >
> > > > > > > Make it work in terms of ctx so that it can be re-used for fdinfo.
> > > > > > >
> > > > > > > Signed-off-by: Rob Clark 
> > > > > > > ---
> > > > > > >   drivers/gpu/drm/msm/adreno/adreno_gpu.c |  4 ++--
> > > > > > >   drivers/gpu/drm/msm/msm_drv.c   |  2 ++
> > > > > > >   drivers/gpu/drm/msm/msm_gpu.c   | 13 ++---
> > > > > > >   drivers/gpu/drm/msm/msm_gpu.h   | 12 ++--
> > > > > > >   drivers/gpu/drm/msm/msm_submitqueue.c   |  1 +
> > > > > > >   5 files changed, 21 insertions(+), 11 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
> > > > > > > b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > > > > > > index bb38e728864d..43c4e1fea83f 100644
> > > > > > > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > > > > > > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > > > > > > @@ -412,7 +412,7 @@ int adreno_set_param(struct msm_gpu *gpu, 
> > > > > > > struct msm_file_private *ctx,
> > > > > > > /* Ensure string is null terminated: */
> > > > > > > str[len] = '\0';
> > > > > > > -   mutex_lock(>lock);
> > > > > > > +   mutex_lock(>lock);
> > > > > > > if (param == MSM_PARAM_COMM) {
> > > > > > > paramp = >comm;
> > > > > > > @@ -423,7 +423,7 @@ int adreno_set_param(struct msm_gpu *gpu, 
> > > > > > > struct msm_file_private *ctx,
> > > > > > > kfree(*paramp);
> > > > > > > *paramp = str;
> > > > > > > -   mutex_unlock(>lock);
> > > > > > > +   mutex_unlock(>lock);
> > > > > > > return 0;
> > > > > > > }
> > > > > > > diff --git a/drivers/gpu/drm/msm/msm_drv.c 
> > > > > > > b/drivers/gpu/drm/msm/msm_drv.c
> > > > > > > index 3d73b98d6a9c..ca0e89e46e13 100644
> > > > > > > --- a/drivers/gpu/drm/msm/msm_drv.c
> > > > > > > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > > > > > > @@ -581,6 +581,8 @@ static int context_init(struct drm_device 
> > > > > > > *dev, struct drm_file *file)
> > > > > > > rwlock_init(>queuelock);
> > > > > > > kref_init(>ref);
> > > > > > > +   ctx->pid = get_pid(task_pid(current));
> > > > > >
> > > > > > Would it simplify things for msm if DRM core had an up to date 
> > > > > > file->pid as
> > > > > > proposed in
> > > > > > https://patchwork.freedesktop.org/patch/526752/?series=109902=4 
> > > > > > ? It
> > > > > > gets updated if ioctl issuer is different than fd opener and this 
> > > > > > being
> > > > > > context_init here reminded me of it. Maybe you wouldn't have to 
> > > > > > track the
> > > > > > pid in msm?
> > > >
> > > > The problem is that we also need this for gpu devcore dumps, which
> > > > could happen after the drm_file is closed.  The ctx can outlive the
> > > > file.
> > > >
> > > I think we all kept forgetting about that. MSM had support for ages,
> > > while AMDGPU is the second driver to land support - just a release
> > > ago.
> > >
> > > > But the ctx->pid has the same problem as the existing file->pid when
> > > > it comes to Xorg.. hopefully over time that problem just goes away.
> > >
> > > Out of curiosity: what do you mean with "when it comes to Xorg" - the
> > > "was_master" handling or something else?
> >
> > The problem is that Xorg is the one to open the drm fd, and then
> > passes the fd to the client.. so the pid of drm_file is the Xorg pid,
> > not the client.  Making it not terribly informative.
> >
> > Tvrtko's patch he linked above would address that for drm_file, but
> > not for other driver internal usages.  Maybe it could be wired up as a
> > helper so that drivers don't have to re-invent that dance.  Idk, I
> > have to think about it.
> >
> > Btw, with my WIP drm sched fence signalling patch lockdep is unhappy
> > when gpu devcore dumps are triggered.  I'm still pondering how to
> > decouple the locking so that anything coming from fs (ie.
> > show_fdinfo()) is decoupled from anything that happens in the fence
> > signaling path.  But will repost this series once I get that sorted
> > out.
>
> So the cleanest imo is that you push most of the capturing into a worker
> that's entirely decoupled. If you have terminal context (i.e. on first
> hang they stop all further cmd submission, which is anyway what
> vk/arb_robustness want), then you don't have to capture at tdr time,
> because there's no subsequent batch that 

Re: [Freedreno] [PATCH v4 02/22] drm/msm/dpu: Remove TE2 block and feature from DPU >= 5.0.0 hardware

2023-04-27 Thread Dmitry Baryshkov

On 27/04/2023 01:37, Marijn Suijten wrote:

No hardware beyond kona (sm8250, DPU 6.0.0) defines the TE2 PINGPONG
sub-block offset downstream, and according to insiders no DPU >= 5.0.0
hardware has support for it either.  Especially since neither downstream
nor upstream utilize these registers in any way, remove the erroneous
specification from SM8150, SC8180X, SM8250, SC7180, SM8350, SC8280XP and
SM8450 to prevent confusion.

Note that downstream enables the PPSPLIT (split-FIFO) topology (single
LM for 2 PP and 2 INTF) based on the presence of a TE2 block.

Fixes: 386fced3f76f ("drm/msm/dpu: add SM8150 to hw catalog")
Fixes: 7bdc0c4b8126 ("msm:disp:dpu1: add support for display for SC7180 target")
Fixes: 0e91bcbb0016 ("drm/msm/dpu: Add SM8350 to hw catalog")
Fixes: 4a352c2fc15a ("drm/msm/dpu: Introduce SC8280XP")
Fixes: 100d7ef6995d ("drm/msm/dpu: add support for SM8450")
Signed-off-by: Marijn Suijten 


Reviewed-by: Dmitry Baryshkov 


---
  .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h |  4 ++--
  .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h|  4 ++--
  .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h |  4 ++--
  .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h |  4 ++--
  .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h |  4 ++--
  .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h   | 24 +++---
  .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h |  4 ++--
  7 files changed, 24 insertions(+), 24 deletions(-)-- 

With best wishes
Dmitry



[Freedreno] [PATCH v2] dt-bindings: display/msm: dsi-controller-main: Document qcom, master-dsi and qcom, sync-dual-dsi

2023-04-27 Thread Jianhua Lu
This fixes warning:
  sm8250-xiaomi-elish-csot.dtb: dsi@ae94000: Unevaluated properties are not 
allowed ('qcom,master-dsi', 'qcom,sync-dual-dsi' were unexpected)

Reviewed-by: Dmitry Baryshkov 
Acked-by: Rob Herring 
Signed-off-by: Jianhua Lu 
---
Changes in v2:
  - pick up tags
  - fix typo (need -> needs)

 .../bindings/display/msm/dsi-controller-main.yaml| 12 
 1 file changed, 12 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml 
b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
index e6c1ebfe8a32..130e16d025bc 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
@@ -82,6 +82,18 @@ properties:
   Indicates if the DSI controller is driving a panel which needs
   2 DSI links.
 
+  qcom,master-dsi:
+type: boolean
+description: |
+  Indicates if the DSI controller is the master DSI controller when
+  qcom,dual-dsi-mode enabled.
+
+  qcom,sync-dual-dsi:
+type: boolean
+description: |
+  Indicates if the DSI controller needs to sync the other DSI controller
+  with MIPI DCS commands when qcom,dual-dsi-mode enabled.
+
   assigned-clocks:
 minItems: 2
 maxItems: 4
-- 
2.39.2



Re: [Freedreno] [PATCH v2 1/3] drm: Create support for Write-Only property blob

2023-04-27 Thread Daniel Vetter
On Fri, Apr 21, 2023 at 12:27:47PM -0400, Mark Yacoub wrote:
> From: Mark Yacoub 
> 
> [Why]
> User space might need to inject data into the kernel without allowing it
> to be read again by any user space.
> An example of where this is particularly useful is secret keys fetched
> by user space and injected into the kernel to enable content protection.
> 
> [How]
> Create a DRM_MODE_CREATE_BLOB_WRITE_ONLY flag used by user space to
> create a blob and mark the blob as write only.
> On reading back the blob, data will be not be copied if it's a write
> only blob

This makes no sense at all, why would you want to disallow reading?
Userspace already knows the key, there's not much point in hiding it from
userspace?

Also for new uapi we need the igt patches and userspace, please link
those.
-Daniel

> 
> Signed-off-by: Mark Yacoub 
> ---
>  drivers/gpu/drm/drm_property.c | 3 ++-
>  include/drm/drm_property.h | 2 ++
>  include/uapi/drm/drm_mode.h| 6 ++
>  3 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
> index dfec479830e49..afedf7109d002 100644
> --- a/drivers/gpu/drm/drm_property.c
> +++ b/drivers/gpu/drm/drm_property.c
> @@ -765,7 +765,7 @@ int drm_mode_getblob_ioctl(struct drm_device *dev,
>   if (!blob)
>   return -ENOENT;
>  
> - if (out_resp->length == blob->length) {
> + if (out_resp->length == blob->length && !blob->is_write_only) {
>   if (copy_to_user(u64_to_user_ptr(out_resp->data),
>blob->data,
>blob->length)) {
> @@ -800,6 +800,7 @@ int drm_mode_createblob_ioctl(struct drm_device *dev,
>   ret = -EFAULT;
>   goto out_blob;
>   }
> + blob->is_write_only = out_resp->flags & DRM_MODE_CREATE_BLOB_WRITE_ONLY;
>  
>   /* Dropping the lock between create_blob and our access here is safe
>* as only the same file_priv can remove the blob; at this point, it is
> diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
> index 65bc9710a4702..700782f021b99 100644
> --- a/include/drm/drm_property.h
> +++ b/include/drm/drm_property.h
> @@ -205,6 +205,7 @@ struct drm_property {
>   *   _mode_config.property_blob_list.
>   * @head_file: entry on the per-file blob list in _file.blobs list.
>   * @length: size of the blob in bytes, invariant over the lifetime of the 
> object
> + * @is_write_only: user space can't read the blob data.
>   * @data: actual data, embedded at the end of this structure
>   *
>   * Blobs are used to store bigger values than what fits directly into the 64
> @@ -219,6 +220,7 @@ struct drm_property_blob {
>   struct list_head head_global;
>   struct list_head head_file;
>   size_t length;
> + bool is_write_only;
>   void *data;
>  };
>  
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 46becedf5b2fc..10403c9a73082 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -1168,6 +1168,9 @@ struct drm_format_modifier {
>   __u64 modifier;
>  };
>  
> +#define DRM_MODE_CREATE_BLOB_WRITE_ONLY  
>   \
> + (1 << 0) /* data of the blob can't be read by user space */
> +
>  /**
>   * struct drm_mode_create_blob - Create New blob property
>   *
> @@ -1181,6 +1184,9 @@ struct drm_mode_create_blob {
>   __u32 length;
>   /** @blob_id: Return: new property ID. */
>   __u32 blob_id;
> + /** Flags for special handling. */
> + __u32 flags;
> + __u32 pad;
>  };
>  
>  /**
> -- 
> 2.40.0.634.g4ca3ef3211-goog
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [Freedreno] [RFC 2/3] drm/msm: Rework get_comm_cmdline() helper

2023-04-27 Thread Daniel Vetter
On Fri, Apr 21, 2023 at 07:47:26AM -0700, Rob Clark wrote:
> On Fri, Apr 21, 2023 at 2:33 AM Emil Velikov  wrote:
> >
> > Greeting all,
> >
> > Sorry for the delay - Easter Holidays, food coma and all that :-)
> >
> > On Tue, 18 Apr 2023 at 15:31, Rob Clark  wrote:
> > >
> > > On Tue, Apr 18, 2023 at 1:34 AM Daniel Vetter  wrote:
> > > >
> > > > On Tue, Apr 18, 2023 at 09:27:49AM +0100, Tvrtko Ursulin wrote:
> > > > >
> > > > > On 17/04/2023 21:12, Rob Clark wrote:
> > > > > > From: Rob Clark 
> > > > > >
> > > > > > Make it work in terms of ctx so that it can be re-used for fdinfo.
> > > > > >
> > > > > > Signed-off-by: Rob Clark 
> > > > > > ---
> > > > > >   drivers/gpu/drm/msm/adreno/adreno_gpu.c |  4 ++--
> > > > > >   drivers/gpu/drm/msm/msm_drv.c   |  2 ++
> > > > > >   drivers/gpu/drm/msm/msm_gpu.c   | 13 ++---
> > > > > >   drivers/gpu/drm/msm/msm_gpu.h   | 12 ++--
> > > > > >   drivers/gpu/drm/msm/msm_submitqueue.c   |  1 +
> > > > > >   5 files changed, 21 insertions(+), 11 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
> > > > > > b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > > > > > index bb38e728864d..43c4e1fea83f 100644
> > > > > > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > > > > > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > > > > > @@ -412,7 +412,7 @@ int adreno_set_param(struct msm_gpu *gpu, 
> > > > > > struct msm_file_private *ctx,
> > > > > > /* Ensure string is null terminated: */
> > > > > > str[len] = '\0';
> > > > > > -   mutex_lock(>lock);
> > > > > > +   mutex_lock(>lock);
> > > > > > if (param == MSM_PARAM_COMM) {
> > > > > > paramp = >comm;
> > > > > > @@ -423,7 +423,7 @@ int adreno_set_param(struct msm_gpu *gpu, 
> > > > > > struct msm_file_private *ctx,
> > > > > > kfree(*paramp);
> > > > > > *paramp = str;
> > > > > > -   mutex_unlock(>lock);
> > > > > > +   mutex_unlock(>lock);
> > > > > > return 0;
> > > > > > }
> > > > > > diff --git a/drivers/gpu/drm/msm/msm_drv.c 
> > > > > > b/drivers/gpu/drm/msm/msm_drv.c
> > > > > > index 3d73b98d6a9c..ca0e89e46e13 100644
> > > > > > --- a/drivers/gpu/drm/msm/msm_drv.c
> > > > > > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > > > > > @@ -581,6 +581,8 @@ static int context_init(struct drm_device *dev, 
> > > > > > struct drm_file *file)
> > > > > > rwlock_init(>queuelock);
> > > > > > kref_init(>ref);
> > > > > > +   ctx->pid = get_pid(task_pid(current));
> > > > >
> > > > > Would it simplify things for msm if DRM core had an up to date 
> > > > > file->pid as
> > > > > proposed in
> > > > > https://patchwork.freedesktop.org/patch/526752/?series=109902=4 ? 
> > > > > It
> > > > > gets updated if ioctl issuer is different than fd opener and this 
> > > > > being
> > > > > context_init here reminded me of it. Maybe you wouldn't have to track 
> > > > > the
> > > > > pid in msm?
> > >
> > > The problem is that we also need this for gpu devcore dumps, which
> > > could happen after the drm_file is closed.  The ctx can outlive the
> > > file.
> > >
> > I think we all kept forgetting about that. MSM had support for ages,
> > while AMDGPU is the second driver to land support - just a release
> > ago.
> >
> > > But the ctx->pid has the same problem as the existing file->pid when
> > > it comes to Xorg.. hopefully over time that problem just goes away.
> >
> > Out of curiosity: what do you mean with "when it comes to Xorg" - the
> > "was_master" handling or something else?
> 
> The problem is that Xorg is the one to open the drm fd, and then
> passes the fd to the client.. so the pid of drm_file is the Xorg pid,
> not the client.  Making it not terribly informative.
> 
> Tvrtko's patch he linked above would address that for drm_file, but
> not for other driver internal usages.  Maybe it could be wired up as a
> helper so that drivers don't have to re-invent that dance.  Idk, I
> have to think about it.
> 
> Btw, with my WIP drm sched fence signalling patch lockdep is unhappy
> when gpu devcore dumps are triggered.  I'm still pondering how to
> decouple the locking so that anything coming from fs (ie.
> show_fdinfo()) is decoupled from anything that happens in the fence
> signaling path.  But will repost this series once I get that sorted
> out.

So the cleanest imo is that you push most of the capturing into a worker
that's entirely decoupled. If you have terminal context (i.e. on first
hang they stop all further cmd submission, which is anyway what
vk/arb_robustness want), then you don't have to capture at tdr time,
because there's no subsequent batch that will wreck the state.

But it only works if your gpu ctx don't have recoverable semantics.

If you can't do that it's a _lot_ of GFP_ATOMIC and trylock and bailing
out if any fails :-/
-Daniel

> 
> BR,
> -R
> 
> >
> > > guess I could do a similar dance to your patch