[Intel-gfx] [PATCH] drm/i915/dsb: modified to drm_info in dsb_prepare()

[Intel-gfx] [PATCH] drm/i915/dsb: modified to drm_info in dsb_prepare()

[Animesh Manna]

The request to aqquire gem resources is failing for DSB in rare
scenario where it is busy and the register programming will be done
through mmio fallback path.

DSB has extra advantage of faster register programming which may
go away through mmio path. Adding wait for gem resource also may
not be right as anyways losing time.

To make the CI execution happy replaced drm_err() to drm_info()
for printing debug info during dsb buffer preparation.

v1: Initial version.
v2: Added print for mmio fallback at out label. [Nirmoy]

Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dsb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index b34a67309976..86c0cf5a03b5 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -283,14 +283,12 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
 
 	obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE);
 	if (IS_ERR(obj)) {
-		drm_err(&i915->drm, "Gem object creation failed\n");
 		kfree(dsb);
 		goto out;
 	}
 
 	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
 	if (IS_ERR(vma)) {
-		drm_err(&i915->drm, "Vma creation failed\n");
 		i915_gem_object_put(obj);
 		kfree(dsb);
 		goto out;
@@ -298,7 +296,6 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
 
 	buf = i915_gem_object_pin_map_unlocked(vma->obj, I915_MAP_WC);
 	if (IS_ERR(buf)) {
-		drm_err(&i915->drm, "Command buffer creation failed\n");
 		i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
 		kfree(dsb);
 		goto out;
@@ -311,6 +308,9 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
 	dsb->ins_start_offset = 0;
 	crtc_state->dsb = dsb;
 out:
+	if (!crtc_state->dsb)
+		drm_info(&i915->drm, "Fallback to mmio for register programming\n");
+
 	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
 }
 
-- 
2.29.0

Re: [Intel-gfx] [PATCH] drm/i915/dsb: modified to drm_info in dsb_prepare()

[Das, Nirmoy]

On 3/25/2022 3:28 PM, Animesh Manna wrote:
> The request to aqquire gem resources is failing for DSB in rare
> scenario where it is busy and the register programming will be done
> through mmio fallback path.
>
> DSB has extra advantage of faster register programming which may
> go away through mmio path. Adding wait for gem resource also may
> not be right as anyways losing time.
>
> To make the CI execution happy replaced drm_err() to drm_info()
> for printing debug info during dsb buffer preparation.
>
> v1: Initial version.
> v2: Added print for mmio fallback at out label. [Nirmoy]
>
> Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dsb.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index b34a67309976..86c0cf5a03b5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -283,14 +283,12 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
>   
>   	obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE);
>   	if (IS_ERR(obj)) {
> -		drm_err(&i915->drm, "Gem object creation failed\n");
>   		kfree(dsb);
>   		goto out;
>   	}
>   
>   	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
>   	if (IS_ERR(vma)) {
> -		drm_err(&i915->drm, "Vma creation failed\n");
>   		i915_gem_object_put(obj);
>   		kfree(dsb);
>   		goto out;
> @@ -298,7 +296,6 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
>   
>   	buf = i915_gem_object_pin_map_unlocked(vma->obj, I915_MAP_WC);
>   	if (IS_ERR(buf)) {
> -		drm_err(&i915->drm, "Command buffer creation failed\n");
>   		i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
>   		kfree(dsb);
>   		goto out;
> @@ -311,6 +308,9 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
>   	dsb->ins_start_offset = 0;
>   	crtc_state->dsb = dsb;
>   out:
> +	if (!crtc_state->dsb)
> +		drm_info(&i915->drm, "Fallback to mmio for register programming\n");

 From the info message, it is hard to know what happened: Something like:

"DSB queue setup failed, will fallback to MMIO for display HW 
programming" should be more understandable.


With such change, this is: Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>


Nirmoy


> +
>   	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
>   }
>   

[Intel-gfx] ✗ Fi.CI.DOCS: warning for drm/i915/dsb: modified to drm_info in dsb_prepare() (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/dsb: modified to drm_info in dsb_prepare() (rev2)
URL   : https://patchwork.freedesktop.org/series/101723/
State : warning

== Summary ==

$ make htmldocs 2>&1 > /dev/null | grep i915
./drivers/gpu/drm/i915/display/intel_drrs.c:1: warning: 'intel_drrs_enable' not found
./drivers/gpu/drm/i915/display/intel_drrs.c:1: warning: 'intel_drrs_disable' not found

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/dsb: modified to drm_info in dsb_prepare() (rev2)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 12686 bytes --]

== Series Details ==

Series: drm/i915/dsb: modified to drm_info in dsb_prepare() (rev2)
URL   : https://patchwork.freedesktop.org/series/101723/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11405 -> Patchwork_22685
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_22685 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_22685, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/index.html

Participating hosts (46 -> 43)
------------------------------

  Additional (1): bat-hsw-1 
  Missing    (4): fi-bsw-cyan bat-rpls-1 shard-tglu fi-bdw-samus 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_22685:

### IGT changes ###

#### Possible regressions ####

  * igt@core_hotunplug@unbind-rebind:
    - fi-cfl-8109u:       NOTRUN -> [INCOMPLETE][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-cfl-8109u/igt@core_hotunplug@unbind-rebind.html

  * igt@gem_tiled_blits@basic:
    - fi-skl-6700k2:      [PASS][2] -> [INCOMPLETE][3]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/fi-skl-6700k2/igt@gem_tiled_blits@basic.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-skl-6700k2/igt@gem_tiled_blits@basic.html
    - fi-cfl-8700k:       [PASS][4] -> [INCOMPLETE][5]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/fi-cfl-8700k/igt@gem_tiled_blits@basic.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-cfl-8700k/igt@gem_tiled_blits@basic.html
    - fi-kbl-7567u:       [PASS][6] -> [INCOMPLETE][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/fi-kbl-7567u/igt@gem_tiled_blits@basic.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-kbl-7567u/igt@gem_tiled_blits@basic.html
    - fi-blb-e6850:       [PASS][8] -> [INCOMPLETE][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/fi-blb-e6850/igt@gem_tiled_blits@basic.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-blb-e6850/igt@gem_tiled_blits@basic.html
    - fi-kbl-8809g:       [PASS][10] -> [INCOMPLETE][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/fi-kbl-8809g/igt@gem_tiled_blits@basic.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-kbl-8809g/igt@gem_tiled_blits@basic.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@core_hotunplug@unbind-rebind:
    - {bat-rpls-2}:       [PASS][12] -> [INCOMPLETE][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/bat-rpls-2/igt@core_hotunplug@unbind-rebind.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/bat-rpls-2/igt@core_hotunplug@unbind-rebind.html
    - {fi-hsw-g3258}:     [PASS][14] -> [INCOMPLETE][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/fi-hsw-g3258/igt@core_hotunplug@unbind-rebind.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-hsw-g3258/igt@core_hotunplug@unbind-rebind.html

  * igt@gem_render_tiled_blits@basic:
    - {fi-tgl-dsi}:       NOTRUN -> [INCOMPLETE][16]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-tgl-dsi/igt@gem_render_tiled_blits@basic.html
    - {fi-ehl-2}:         [PASS][17] -> [INCOMPLETE][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/fi-ehl-2/igt@gem_render_tiled_blits@basic.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-ehl-2/igt@gem_render_tiled_blits@basic.html

  
Known issues
------------

  Here are the changes found in Patchwork_22685 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-gfx:
    - fi-hsw-4770:        NOTRUN -> [SKIP][19] ([fdo#109271] / [fdo#109315]) +17 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-hsw-4770/igt@amdgpu/amd_basic@cs-gfx.html

  * igt@amdgpu/amd_basic@query-info:
    - fi-glk-dsi:         NOTRUN -> [SKIP][20] ([fdo#109271]) +17 similar issues
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-glk-dsi/igt@amdgpu/amd_basic@query-info.html
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][21] ([fdo#109315])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-tgl-1115g4/igt@amdgpu/amd_basic@query-info.html

  * igt@amdgpu/amd_cs_nop@nop-gfx0:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][22] ([fdo#109315] / [i915#2575]) +16 similar issues
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-tgl-1115g4/igt@amdgpu/amd_cs_nop@nop-gfx0.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-hsw-4770:        NOTRUN -> [SKIP][23] ([fdo#109271] / [i915#3012])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-hsw-4770/igt@i915_pm_backlight@basic-brightness.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-hsw-4770:        NOTRUN -> [SKIP][24] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-hsw-4770/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-cfl-8109u:       NOTRUN -> [SKIP][25] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-cfl-8109u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c:
    - fi-bwr-2160:        NOTRUN -> [SKIP][26] ([fdo#109271] / [i915#5341])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-bwr-2160/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-hsw-4770:        NOTRUN -> [SKIP][27] ([fdo#109271] / [i915#533])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-hsw-4770/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html
    - fi-cfl-8109u:       NOTRUN -> [SKIP][28] ([fdo#109271] / [i915#533])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-cfl-8109u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-c:
    - fi-bwr-2160:        NOTRUN -> [SKIP][29] ([fdo#109271]) +45 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-bwr-2160/igt@kms_pipe_crc_basic@read-crc-pipe-c.html

  * igt@kms_psr@primary_mmap_gtt:
    - fi-cfl-8109u:       NOTRUN -> [SKIP][30] ([fdo#109271]) +9 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-cfl-8109u/igt@kms_psr@primary_mmap_gtt.html
    - fi-hsw-4770:        NOTRUN -> [SKIP][31] ([fdo#109271] / [i915#1072]) +3 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-hsw-4770/igt@kms_psr@primary_mmap_gtt.html

  * igt@prime_vgem@basic-userptr:
    - fi-hsw-4770:        NOTRUN -> [SKIP][32] ([fdo#109271]) +2 similar issues
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-hsw-4770/igt@prime_vgem@basic-userptr.html

  
#### Possible fixes ####

  * igt@core_hotunplug@unbind-rebind:
    - fi-tgl-1115g4:      [INCOMPLETE][33] ([i915#1373]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/fi-tgl-1115g4/igt@core_hotunplug@unbind-rebind.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-tgl-1115g4/igt@core_hotunplug@unbind-rebind.html
    - fi-glk-dsi:         [INCOMPLETE][35] -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/fi-glk-dsi/igt@core_hotunplug@unbind-rebind.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-glk-dsi/igt@core_hotunplug@unbind-rebind.html

  * igt@gem_exec_suspend@basic-s3@smem:
    - {fi-rkl-11600}:     [INCOMPLETE][37] ([i915#5127]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/fi-rkl-11600/igt@gem_exec_suspend@basic-s3@smem.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-rkl-11600/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@gem_render_linear_blits@basic:
    - {fi-tgl-dsi}:       [INCOMPLETE][39] -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/fi-tgl-dsi/igt@gem_render_linear_blits@basic.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-tgl-dsi/igt@gem_render_linear_blits@basic.html

  * igt@gem_render_tiled_blits@basic:
    - fi-cfl-8109u:       [INCOMPLETE][41] -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/fi-cfl-8109u/igt@gem_render_tiled_blits@basic.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-cfl-8109u/igt@gem_render_tiled_blits@basic.html

  * igt@gem_tiled_blits@basic:
    - fi-hsw-4770:        [INCOMPLETE][43] -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/fi-hsw-4770/igt@gem_tiled_blits@basic.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-hsw-4770/igt@gem_tiled_blits@basic.html
    - fi-bwr-2160:        [INCOMPLETE][45] -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/fi-bwr-2160/igt@gem_tiled_blits@basic.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-bwr-2160/igt@gem_tiled_blits@basic.html

  * igt@kms_busy@basic@flip:
    - {bat-adlp-6}:       [DMESG-WARN][47] ([i915#3576]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/bat-adlp-6/igt@kms_busy@basic@flip.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/bat-adlp-6/igt@kms_busy@basic@flip.html

  
#### Warnings ####

  * igt@runner@aborted:
    - fi-blb-e6850:       [FAIL][49] ([fdo#109271] / [i915#2403] / [i915#4312]) -> [FAIL][50] ([i915#2403] / [i915#4312])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11405/fi-blb-e6850/igt@runner@aborted.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/fi-blb-e6850/igt@runner@aborted.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1373]: https://gitlab.freedesktop.org/drm/intel/issues/1373
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2403]: https://gitlab.freedesktop.org/drm/intel/issues/2403
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#3012]: https://gitlab.freedesktop.org/drm/intel/issues/3012
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5127]: https://gitlab.freedesktop.org/drm/intel/issues/5127
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5341]: https://gitlab.freedesktop.org/drm/intel/issues/5341


Build changes
-------------

  * Linux: CI_DRM_11405 -> Patchwork_22685

  CI-20190529: 20190529
  CI_DRM_11405: 631579c25b67cc4a6ee014440ddcac6c72b9bf50 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6393: 1d267ea1b829fa10b31a37ccf6b4a970f032784a @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_22685: a701ae0c8efe1a07a3087fa1bfdb464824a36310 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a701ae0c8efe drm/i915/dsb: modified to drm_info in dsb_prepare()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22685/index.html

[-- Attachment #2: Type: text/html, Size: 14543 bytes --]

[Intel-gfx] [PATCH] drm/i915/dsb: modified to drm_info in dsb_prepare()

[Animesh Manna]

The request to aqquire gem resources is failing for DSB in rare
scenario where it is busy and the register programming will be done
through mmio fallback path.

DSB has extra advantage of faster register programming which may
go away through mmio path. Adding wait for gem resource also may
not be right as anyways losing time.

To make the CI execution happy replaced drm_err() to drm_info()
for printing debug info during dsb buffer preparation.

v1: Initial version.
v2: Added print for mmio fallback at out label. [Nirmoy]
v3: Improved debug message. [Nirmoy]

Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>
Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dsb.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index b34a67309976..c4affcb216fd 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -283,14 +283,12 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
 
 	obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE);
 	if (IS_ERR(obj)) {
-		drm_err(&i915->drm, "Gem object creation failed\n");
 		kfree(dsb);
 		goto out;
 	}
 
 	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
 	if (IS_ERR(vma)) {
-		drm_err(&i915->drm, "Vma creation failed\n");
 		i915_gem_object_put(obj);
 		kfree(dsb);
 		goto out;
@@ -298,7 +296,6 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
 
 	buf = i915_gem_object_pin_map_unlocked(vma->obj, I915_MAP_WC);
 	if (IS_ERR(buf)) {
-		drm_err(&i915->drm, "Command buffer creation failed\n");
 		i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
 		kfree(dsb);
 		goto out;
@@ -311,6 +308,10 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
 	dsb->ins_start_offset = 0;
 	crtc_state->dsb = dsb;
 out:
+	if (!crtc_state->dsb)
+		drm_info(&i915->drm,
+			 "DSB queue setup failed, will fallback to MMIO for display HW programming\n");
+
 	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
 }
 
-- 
2.29.0

Re: [Intel-gfx] [PATCH] drm/i915/dsb: modified to drm_info in dsb_prepare()

[Shankar, Uma]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Animesh
> Manna
> Sent: Friday, March 25, 2022 9:42 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Das, Nirmoy <nirmoy.das@intel.com>
> Subject: [Intel-gfx] [PATCH] drm/i915/dsb: modified to drm_info in dsb_prepare()
> 
> The request to aqquire gem resources is failing for DSB in rare scenario where it is
> busy and the register programming will be done through mmio fallback path.
> 
> DSB has extra advantage of faster register programming which may go away
> through mmio path. Adding wait for gem resource also may not be right as anyways
> losing time.
> 
> To make the CI execution happy replaced drm_err() to drm_info() for printing debug
> info during dsb buffer preparation.
> 
> v1: Initial version.
> v2: Added print for mmio fallback at out label. [Nirmoy]
> v3: Improved debug message. [Nirmoy]

Pushed to drm-intel-next. Thanks for the patch and reviews.

Regards,
Uma Shankar

> Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>
> Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dsb.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c
> b/drivers/gpu/drm/i915/display/intel_dsb.c
> index b34a67309976..c4affcb216fd 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -283,14 +283,12 @@ void intel_dsb_prepare(struct intel_crtc_state
> *crtc_state)
> 
>  	obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE);
>  	if (IS_ERR(obj)) {
> -		drm_err(&i915->drm, "Gem object creation failed\n");
>  		kfree(dsb);
>  		goto out;
>  	}
> 
>  	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
>  	if (IS_ERR(vma)) {
> -		drm_err(&i915->drm, "Vma creation failed\n");
>  		i915_gem_object_put(obj);
>  		kfree(dsb);
>  		goto out;
> @@ -298,7 +296,6 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
> 
>  	buf = i915_gem_object_pin_map_unlocked(vma->obj, I915_MAP_WC);
>  	if (IS_ERR(buf)) {
> -		drm_err(&i915->drm, "Command buffer creation failed\n");
>  		i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
>  		kfree(dsb);
>  		goto out;
> @@ -311,6 +308,10 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
>  	dsb->ins_start_offset = 0;
>  	crtc_state->dsb = dsb;
>  out:
> +	if (!crtc_state->dsb)
> +		drm_info(&i915->drm,
> +			 "DSB queue setup failed, will fallback to MMIO for display
> HW
> +programming\n");
> +
>  	intel_runtime_pm_put(&i915->runtime_pm, wakeref);  }
> 
> --
> 2.29.0

[Intel-gfx] [PATCH] drm/i915/dsb: modified to drm_info in dsb_prepare()

[Animesh Manna]

The request to aqquire gem resources is failing for DSB in rare
scenario where it is busy and the register programming will be done
through mmio fallback path.

DSB has extra advantage of faster register programming which may
go away through mmio path. Adding wait for gem resource also may
not be right as anyways losing time.

To make the CI execution happy replaced drm_dbg_kms() to drm_info()
for printing debuf info during dsb buffer preparation.

Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dsb.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index b34a67309976..b68dd7bd5271 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -275,7 +275,7 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
 
 	dsb = kmalloc(sizeof(*dsb), GFP_KERNEL);
 	if (!dsb) {
-		drm_err(&i915->drm, "DSB object creation failed\n");
+		drm_info(&i915->drm, "DSB object creation failed\n");
 		return;
 	}
 
@@ -283,14 +283,14 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
 
 	obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE);
 	if (IS_ERR(obj)) {
-		drm_err(&i915->drm, "Gem object creation failed\n");
+		drm_info(&i915->drm, "Gem object creation failed\n");
 		kfree(dsb);
 		goto out;
 	}
 
 	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
 	if (IS_ERR(vma)) {
-		drm_err(&i915->drm, "Vma creation failed\n");
+		drm_info(&i915->drm, "Vma creation failed\n");
 		i915_gem_object_put(obj);
 		kfree(dsb);
 		goto out;
@@ -298,7 +298,7 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
 
 	buf = i915_gem_object_pin_map_unlocked(vma->obj, I915_MAP_WC);
 	if (IS_ERR(buf)) {
-		drm_err(&i915->drm, "Command buffer creation failed\n");
+		drm_info(&i915->drm, "Command buffer creation failed\n");
 		i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
 		kfree(dsb);
 		goto out;
-- 
2.29.0

Re: [Intel-gfx] [PATCH] drm/i915/dsb: modified to drm_info in dsb_prepare()

[Das, Nirmoy]

On 3/24/2022 8:43 AM, Animesh Manna wrote:
> The request to aqquire gem resources is failing for DSB in rare
> scenario where it is busy and the register programming will be done
> through mmio fallback path.
>
> DSB has extra advantage of faster register programming which may
> go away through mmio path. Adding wait for gem resource also may
> not be right as anyways losing time.
>
> To make the CI execution happy replaced drm_dbg_kms() to drm_info()
> for printing debuf info during dsb buffer preparation.
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dsb.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index b34a67309976..b68dd7bd5271 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -275,7 +275,7 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
>   
>   	dsb = kmalloc(sizeof(*dsb), GFP_KERNEL);
>   	if (!dsb) {
> -		drm_err(&i915->drm, "DSB object creation failed\n");
> +		drm_info(&i915->drm, "DSB object creation failed\n");
>   		return;
>   	}
>   
> @@ -283,14 +283,14 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
>   
>   	obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE);
>   	if (IS_ERR(obj)) {
> -		drm_err(&i915->drm, "Gem object creation failed\n");
> +		drm_info(&i915->drm, "Gem object creation failed\n");

If CI is happy with drm_warn then it makes sense to use drm_warn.


>   		kfree(dsb);
>   		goto out;
>   	}
>   
>   	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
>   	if (IS_ERR(vma)) {
> -		drm_err(&i915->drm, "Vma creation failed\n");
> +		drm_info(&i915->drm, "Vma creation failed\n");


These messages are bit vague, add "DSB VMA creation failed" or something 
similar.

With that Acked-by: Nirmoy Das <nirmoy.das@intel.com>


Nirmoy


>   		i915_gem_object_put(obj);
>   		kfree(dsb);
>   		goto out;
> @@ -298,7 +298,7 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
>   
>   	buf = i915_gem_object_pin_map_unlocked(vma->obj, I915_MAP_WC);
>   	if (IS_ERR(buf)) {
> -		drm_err(&i915->drm, "Command buffer creation failed\n");
> +		drm_info(&i915->drm, "Command buffer creation failed\n");
>   		i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
>   		kfree(dsb);
>   		goto out;

Re: [Intel-gfx] [PATCH] drm/i915/dsb: modified to drm_info in dsb_prepare()

[Manna, Animesh]

> -----Original Message-----
> From: Das, Nirmoy <nirmoy.das@linux.intel.com>
> Sent: Thursday, March 24, 2022 2:39 PM
> To: Manna, Animesh <animesh.manna@intel.com>; intel-
> gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/dsb: modified to drm_info in
> dsb_prepare()
> 
> 
> On 3/24/2022 8:43 AM, Animesh Manna wrote:
> > The request to aqquire gem resources is failing for DSB in rare
> > scenario where it is busy and the register programming will be done
> > through mmio fallback path.
> >
> > DSB has extra advantage of faster register programming which may go
> > away through mmio path. Adding wait for gem resource also may not be
> > right as anyways losing time.
> >
> > To make the CI execution happy replaced drm_dbg_kms() to drm_info()
> > for printing debuf info during dsb buffer preparation.
> >
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > ---
> >   drivers/gpu/drm/i915/display/intel_dsb.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c
> > b/drivers/gpu/drm/i915/display/intel_dsb.c
> > index b34a67309976..b68dd7bd5271 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> > @@ -275,7 +275,7 @@ void intel_dsb_prepare(struct intel_crtc_state
> > *crtc_state)
> >
> >   	dsb = kmalloc(sizeof(*dsb), GFP_KERNEL);
> >   	if (!dsb) {
> > -		drm_err(&i915->drm, "DSB object creation failed\n");
> > +		drm_info(&i915->drm, "DSB object creation failed\n");
> >   		return;
> >   	}
> >
> > @@ -283,14 +283,14 @@ void intel_dsb_prepare(struct intel_crtc_state
> > *crtc_state)
> >
> >   	obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE);
> >   	if (IS_ERR(obj)) {
> > -		drm_err(&i915->drm, "Gem object creation failed\n");
> > +		drm_info(&i915->drm, "Gem object creation failed\n");
> 
> If CI is happy with drm_warn then it makes sense to use drm_warn.

Checked with CI team, seems drm_warn also considered as bug, is it ok to use drm_info?

> 
> 
> >   		kfree(dsb);
> >   		goto out;
> >   	}
> >
> >   	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
> >   	if (IS_ERR(vma)) {
> > -		drm_err(&i915->drm, "Vma creation failed\n");
> > +		drm_info(&i915->drm, "Vma creation failed\n");
> 
> 
> These messages are bit vague, add "DSB VMA creation failed" or something
> similar.
> 
> With that Acked-by: Nirmoy Das <nirmoy.das@intel.com>

Thanks for review, will modify in next version.

Regards,
Animesh
 
> 
> 
> Nirmoy
> 
> 
> >   		i915_gem_object_put(obj);
> >   		kfree(dsb);
> >   		goto out;
> > @@ -298,7 +298,7 @@ void intel_dsb_prepare(struct intel_crtc_state
> > *crtc_state)
> >
> >   	buf = i915_gem_object_pin_map_unlocked(vma->obj, I915_MAP_WC);
> >   	if (IS_ERR(buf)) {
> > -		drm_err(&i915->drm, "Command buffer creation failed\n");
> > +		drm_info(&i915->drm, "Command buffer creation failed\n");
> >   		i915_vma_unpin_and_release(&vma,
> I915_VMA_RELEASE_MAP);
> >   		kfree(dsb);
> >   		goto out;

Re: [Intel-gfx] [PATCH] drm/i915/dsb: modified to drm_info in dsb_prepare()

[Das, Nirmoy]

On 3/25/2022 2:22 PM, Manna, Animesh wrote:
>
>> -----Original Message-----
>> From: Das, Nirmoy <nirmoy.das@linux.intel.com>
>> Sent: Thursday, March 24, 2022 2:39 PM
>> To: Manna, Animesh <animesh.manna@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/dsb: modified to drm_info in
>> dsb_prepare()
>>
>>
>> On 3/24/2022 8:43 AM, Animesh Manna wrote:
>>> The request to aqquire gem resources is failing for DSB in rare
>>> scenario where it is busy and the register programming will be done
>>> through mmio fallback path.
>>>
>>> DSB has extra advantage of faster register programming which may go
>>> away through mmio path. Adding wait for gem resource also may not be
>>> right as anyways losing time.
>>>
>>> To make the CI execution happy replaced drm_dbg_kms() to drm_info()
>>> for printing debuf info during dsb buffer preparation.
>>>
>>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/display/intel_dsb.c | 8 ++++----
>>>    1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c
>>> b/drivers/gpu/drm/i915/display/intel_dsb.c
>>> index b34a67309976..b68dd7bd5271 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
>>> @@ -275,7 +275,7 @@ void intel_dsb_prepare(struct intel_crtc_state
>>> *crtc_state)
>>>
>>>    	dsb = kmalloc(sizeof(*dsb), GFP_KERNEL);
>>>    	if (!dsb) {
>>> -		drm_err(&i915->drm, "DSB object creation failed\n");
>>> +		drm_info(&i915->drm, "DSB object creation failed\n");
>>>    		return;
>>>    	}
>>>
>>> @@ -283,14 +283,14 @@ void intel_dsb_prepare(struct intel_crtc_state
>>> *crtc_state)
>>>
>>>    	obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE);
>>>    	if (IS_ERR(obj)) {
>>> -		drm_err(&i915->drm, "Gem object creation failed\n");
>>> +		drm_info(&i915->drm, "Gem object creation failed\n");
>> If CI is happy with drm_warn then it makes sense to use drm_warn.
> Checked with CI team, seems drm_warn also considered as bug, is it ok to use drm_info?


In that case: don't print anything on each error and at out label,  you 
can print an info about the fallback option.


Nirmoy

>
>>
>>>    		kfree(dsb);
>>>    		goto out;
>>>    	}
>>>
>>>    	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
>>>    	if (IS_ERR(vma)) {
>>> -		drm_err(&i915->drm, "Vma creation failed\n");
>>> +		drm_info(&i915->drm, "Vma creation failed\n");
>>
>> These messages are bit vague, add "DSB VMA creation failed" or something
>> similar.
>>
>> With that Acked-by: Nirmoy Das <nirmoy.das@intel.com>
> Thanks for review, will modify in next version.
>
> Regards,
> Animesh
>   
>>
>> Nirmoy
>>
>>
>>>    		i915_gem_object_put(obj);
>>>    		kfree(dsb);
>>>    		goto out;
>>> @@ -298,7 +298,7 @@ void intel_dsb_prepare(struct intel_crtc_state
>>> *crtc_state)
>>>
>>>    	buf = i915_gem_object_pin_map_unlocked(vma->obj, I915_MAP_WC);
>>>    	if (IS_ERR(buf)) {
>>> -		drm_err(&i915->drm, "Command buffer creation failed\n");
>>> +		drm_info(&i915->drm, "Command buffer creation failed\n");
>>>    		i915_vma_unpin_and_release(&vma,
>> I915_VMA_RELEASE_MAP);
>>>    		kfree(dsb);
>>>    		goto out;