[Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in DSC disable

[Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in DSC disable

[Vandita Kulkarni]

There can be a chance that pre os has enabled
DSC and driver's compute config would not need
dsc to be enabled, in such case if we check on
compute config's compression state to disable,
we might end up in state mismatch.

Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 drivers/gpu/drm/i915/display/intel_vdsc.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
index 19cd9531c115..b05a96011d93 100644
--- a/drivers/gpu/drm/i915/display/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
@@ -1161,10 +1161,6 @@ void intel_dsc_disable(const struct intel_crtc_state *old_crtc_state)
 	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 
-	if (!(old_crtc_state->dsc.compression_enable &&
-	      old_crtc_state->bigjoiner))
-		return;
-
 	intel_de_write(dev_priv, dss_ctl1_reg(old_crtc_state), 0);
 	intel_de_write(dev_priv, dss_ctl2_reg(old_crtc_state), 0);
 }
-- 
2.21.0.5.gaeb582a

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in DSC disable

[Saarinen, Jani]

Hi, 
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Vandita
> Kulkarni
> Sent: torstai 3. kesäkuuta 2021 9.54
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>
> Subject: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in DSC disable
> 
> There can be a chance that pre os has enabled DSC and driver's compute config
> would not need dsc to be enabled, in such case if we check on compute config's
> compression state to disable, we might end up in state mismatch.

I assume this fixes real gitlab issue too? 

> 
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vdsc.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 19cd9531c115..b05a96011d93 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -1161,10 +1161,6 @@ void intel_dsc_disable(const struct intel_crtc_state
> *old_crtc_state)
>  	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> 
> -	if (!(old_crtc_state->dsc.compression_enable &&
> -	      old_crtc_state->bigjoiner))
> -		return;
> -
>  	intel_de_write(dev_priv, dss_ctl1_reg(old_crtc_state), 0);
>  	intel_de_write(dev_priv, dss_ctl2_reg(old_crtc_state), 0);  }
> --
> 2.21.0.5.gaeb582a
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/dsc: Remove redundant checks in DSC disable

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 12727 bytes --]

== Series Details ==

Series: drm/i915/dsc: Remove redundant checks in DSC disable
URL   : https://patchwork.freedesktop.org/series/90925/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_10162 -> Patchwork_20270
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_20270 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_20270, 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_20270/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_chamelium@dp-crc-fast:
    - fi-icl-u2:          [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-icl-u2/igt@kms_chamelium@dp-crc-fast.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-icl-u2/igt@kms_chamelium@dp-crc-fast.html

  
#### Warnings ####

  * igt@runner@aborted:
    - fi-bdw-5557u:       [FAIL][3] ([i915#3462]) -> [FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-bdw-5557u/igt@runner@aborted.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-bdw-5557u/igt@runner@aborted.html

  
#### Suppressed ####

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

  * igt@kms_flip@basic-flip-vs-dpms@a-hdmi-a2:
    - {fi-ehl-1}:         [PASS][5] -> [DMESG-WARN][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-ehl-1/igt@kms_flip@basic-flip-vs-dpms@a-hdmi-a2.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-ehl-1/igt@kms_flip@basic-flip-vs-dpms@a-hdmi-a2.html

  
New tests
---------

  New tests have been introduced between CI_DRM_10162 and Patchwork_20270:

### New IGT tests (16) ###

  * igt@kms_flip@basic-flip-vs-dpms@a-dp3:
    - Statuses : 1 pass(s)
    - Exec time: [0.69] s

  * igt@kms_flip@basic-flip-vs-dpms@b-dp3:
    - Statuses : 1 pass(s)
    - Exec time: [0.77] s

  * igt@kms_flip@basic-flip-vs-dpms@c-dp3:
    - Statuses : 1 pass(s)
    - Exec time: [0.78] s

  * igt@kms_flip@basic-flip-vs-dpms@d-dp3:
    - Statuses : 1 pass(s)
    - Exec time: [0.77] s

  * igt@kms_flip@basic-flip-vs-modeset@a-dp3:
    - Statuses : 1 pass(s)
    - Exec time: [0.69] s

  * igt@kms_flip@basic-flip-vs-modeset@b-dp3:
    - Statuses : 1 pass(s)
    - Exec time: [0.77] s

  * igt@kms_flip@basic-flip-vs-modeset@c-dp3:
    - Statuses : 1 pass(s)
    - Exec time: [0.77] s

  * igt@kms_flip@basic-flip-vs-modeset@d-dp3:
    - Statuses : 1 pass(s)
    - Exec time: [0.79] s

  * igt@kms_flip@basic-flip-vs-wf_vblank@a-dp3:
    - Statuses : 1 pass(s)
    - Exec time: [1.01] s

  * igt@kms_flip@basic-flip-vs-wf_vblank@b-dp3:
    - Statuses : 1 pass(s)
    - Exec time: [0.98] s

  * igt@kms_flip@basic-flip-vs-wf_vblank@c-dp3:
    - Statuses : 1 pass(s)
    - Exec time: [0.98] s

  * igt@kms_flip@basic-flip-vs-wf_vblank@d-dp3:
    - Statuses : 1 pass(s)
    - Exec time: [0.97] s

  * igt@kms_flip@basic-plain-flip@a-dp3:
    - Statuses : 1 pass(s)
    - Exec time: [0.73] s

  * igt@kms_flip@basic-plain-flip@b-dp3:
    - Statuses : 1 pass(s)
    - Exec time: [0.70] s

  * igt@kms_flip@basic-plain-flip@c-dp3:
    - Statuses : 1 pass(s)
    - Exec time: [0.69] s

  * igt@kms_flip@basic-plain-flip@d-dp3:
    - Statuses : 1 pass(s)
    - Exec time: [0.69] s

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_force_connector_basic@force-connector-state:
    - fi-kbl-guc:         [PASS][7] -> [DMESG-WARN][8] ([i915#1436])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-kbl-guc/igt@kms_force_connector_basic@force-connector-state.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-kbl-guc/igt@kms_force_connector_basic@force-connector-state.html

  
#### Possible fixes ####

  * igt@kms_flip@basic-flip-vs-modeset@a-edp1:
    - fi-tgl-y:           [DMESG-WARN][9] ([i915#1982]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-tgl-y/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-tgl-y/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html

  
#### Warnings ####

  * igt@i915_selftest@live@execlists:
    - fi-bsw-nick:        [DMESG-FAIL][11] ([i915#3462]) -> [INCOMPLETE][12] ([i915#2782] / [i915#2940] / [i915#3462])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-bsw-nick/igt@i915_selftest@live@execlists.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-bsw-nick/igt@i915_selftest@live@execlists.html
    - fi-tgl-u2:          [DMESG-FAIL][13] ([i915#3462]) -> [INCOMPLETE][14] ([i915#3462])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-tgl-u2/igt@i915_selftest@live@execlists.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-tgl-u2/igt@i915_selftest@live@execlists.html

  * igt@runner@aborted:
    - fi-kbl-x1275:       [FAIL][15] ([i915#1436] / [i915#3363]) -> [FAIL][16] ([i915#1569] / [i915#192] / [i915#193] / [i915#194] / [i915#3363])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-kbl-x1275/igt@runner@aborted.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-kbl-x1275/igt@runner@aborted.html
    - fi-cfl-8700k:       [FAIL][17] ([i915#3363]) -> [FAIL][18] ([i915#3363] / [k.org#202107] / [k.org#202109])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-cfl-8700k/igt@runner@aborted.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-cfl-8700k/igt@runner@aborted.html
    - fi-skl-6600u:       [FAIL][19] ([i915#1436] / [i915#3363]) -> [FAIL][20] ([i915#3363])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-skl-6600u/igt@runner@aborted.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-skl-6600u/igt@runner@aborted.html
    - fi-cfl-8109u:       [FAIL][21] ([i915#3363]) -> [FAIL][22] ([i915#3363] / [k.org#202107] / [k.org#202109])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-cfl-8109u/igt@runner@aborted.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-cfl-8109u/igt@runner@aborted.html
    - fi-icl-u2:          [FAIL][23] ([i915#2782] / [i915#3363]) -> [FAIL][24] ([i915#1814] / [i915#3363])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-icl-u2/igt@runner@aborted.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-icl-u2/igt@runner@aborted.html
    - fi-apl-guc:         [FAIL][25] ([i915#2426] / [i915#3363]) -> [FAIL][26] ([i915#1610] / [i915#3363])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-apl-guc/igt@runner@aborted.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-apl-guc/igt@runner@aborted.html
    - fi-kbl-r:           [FAIL][27] ([i915#1436] / [i915#3363]) -> [FAIL][28] ([i915#1569] / [i915#192] / [i915#193] / [i915#194] / [i915#3363])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-kbl-r/igt@runner@aborted.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-kbl-r/igt@runner@aborted.html
    - fi-kbl-soraka:      [FAIL][29] ([i915#1436] / [i915#2426] / [i915#3363]) -> [FAIL][30] ([i915#1569] / [i915#192] / [i915#193] / [i915#194] / [i915#3363])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-kbl-soraka/igt@runner@aborted.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-kbl-soraka/igt@runner@aborted.html
    - fi-kbl-7500u:       [FAIL][31] ([i915#1436] / [i915#3363]) -> [FAIL][32] ([i915#1569] / [i915#192] / [i915#193] / [i915#194] / [i915#3363])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-kbl-7500u/igt@runner@aborted.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-kbl-7500u/igt@runner@aborted.html
    - fi-kbl-guc:         [FAIL][33] ([i915#1436] / [i915#3363]) -> [FAIL][34] ([i915#3363])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-kbl-guc/igt@runner@aborted.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-kbl-guc/igt@runner@aborted.html
    - fi-cml-u2:          [FAIL][35] ([i915#3363] / [i915#3462]) -> [FAIL][36] ([i915#3363])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-cml-u2/igt@runner@aborted.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-cml-u2/igt@runner@aborted.html
    - fi-cml-s:           [FAIL][37] ([i915#3363] / [i915#3462]) -> [FAIL][38] ([i915#3363])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-cml-s/igt@runner@aborted.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-cml-s/igt@runner@aborted.html
    - fi-cfl-guc:         [FAIL][39] ([i915#3363]) -> [FAIL][40] ([i915#3363] / [k.org#202107] / [k.org#202109])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-cfl-guc/igt@runner@aborted.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-cfl-guc/igt@runner@aborted.html
    - fi-kbl-7567u:       [FAIL][41] ([i915#1436] / [i915#3363]) -> [FAIL][42] ([fdo#109271] / [i915#1814] / [i915#3363])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-kbl-7567u/igt@runner@aborted.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-kbl-7567u/igt@runner@aborted.html
    - fi-skl-6700k2:      [FAIL][43] ([i915#1436] / [i915#3363]) -> [FAIL][44] ([i915#3363])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10162/fi-skl-6700k2/igt@runner@aborted.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20270/fi-skl-6700k2/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#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#1569]: https://gitlab.freedesktop.org/drm/intel/issues/1569
  [i915#1610]: https://gitlab.freedesktop.org/drm/intel/issues/1610
  [i915#1814]: https://gitlab.freedesktop.org/drm/intel/issues/1814
  [i915#192]: https://gitlab.freedesktop.org/drm/intel/issues/192
  [i915#193]: https://gitlab.freedesktop.org/drm/intel/issues/193
  [i915#194]: https://gitlab.freedesktop.org/drm/intel/issues/194
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2426]: https://gitlab.freedesktop.org/drm/intel/issues/2426
  [i915#2505]: https://gitlab.freedesktop.org/drm/intel/issues/2505
  [i915#2782]: https://gitlab.freedesktop.org/drm/intel/issues/2782
  [i915#2940]: https://gitlab.freedesktop.org/drm/intel/issues/2940
  [i915#2966]: https://gitlab.freedesktop.org/drm/intel/issues/2966
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3363]: https://gitlab.freedesktop.org/drm/intel/issues/3363
  [i915#3462]: https://gitlab.freedesktop.org/drm/intel/issues/3462
  [i915#3537]: https://gitlab.freedesktop.org/drm/intel/issues/3537
  [k.org#202107]: https://bugzilla.kernel.org/show_bug.cgi?id=202107
  [k.org#202109]: https://bugzilla.kernel.org/show_bug.cgi?id=202109


Participating hosts (46 -> 42)
------------------------------

  Missing    (4): fi-ilk-m540 fi-bsw-cyan fi-bdw-samus fi-hsw-4200u 


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

  * Linux: CI_DRM_10162 -> Patchwork_20270

  CI-20190529: 20190529
  CI_DRM_10162: 14fc8c382389c84c90e7a21f01bd67513daa9778 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6098: 1fbc1e7d602f96a7f4e2b95057eef994656b8e74 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_20270: 1a1faaa72cc144e72e93e41a555983eb73abc378 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

1a1faaa72cc1 drm/i915/dsc: Remove redundant checks in DSC disable

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 17581 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in DSC disable

[Kulkarni, Vandita]

> -----Original Message-----
> From: Saarinen, Jani <jani.saarinen@intel.com>
> Sent: Thursday, June 3, 2021 1:07 PM
> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in
> DSC disable
> 
> Hi,
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of
> > Vandita Kulkarni
> > Sent: torstai 3. kesäkuuta 2021 9.54
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Nikula, Jani <jani.nikula@intel.com>
> > Subject: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in
> > DSC disable
> >
> > There can be a chance that pre os has enabled DSC and driver's compute
> > config would not need dsc to be enabled, in such case if we check on
> > compute config's compression state to disable, we might end up in state
> mismatch.
> 
> I assume this fixes real gitlab issue too?
Okay, will add the tag 
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3537

Thanks,
Vandita
> 
> >
> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_vdsc.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > index 19cd9531c115..b05a96011d93 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > @@ -1161,10 +1161,6 @@ void intel_dsc_disable(const struct
> > intel_crtc_state
> > *old_crtc_state)
> >  	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc);
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >
> > -	if (!(old_crtc_state->dsc.compression_enable &&
> > -	      old_crtc_state->bigjoiner))
> > -		return;
> > -
> >  	intel_de_write(dev_priv, dss_ctl1_reg(old_crtc_state), 0);
> >  	intel_de_write(dev_priv, dss_ctl2_reg(old_crtc_state), 0);  }
> > --
> > 2.21.0.5.gaeb582a
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in DSC disable

[Jani Nikula]

On Thu, 03 Jun 2021, "Kulkarni, Vandita" <vandita.kulkarni@intel.com> wrote:
>> -----Original Message-----
>> From: Saarinen, Jani <jani.saarinen@intel.com>
>> Sent: Thursday, June 3, 2021 1:07 PM
>> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Nikula, Jani <jani.nikula@intel.com>
>> Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in
>> DSC disable
>> 
>> Hi,
>> > -----Original Message-----
>> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of
>> > Vandita Kulkarni
>> > Sent: torstai 3. kesäkuuta 2021 9.54
>> > To: intel-gfx@lists.freedesktop.org
>> > Cc: Nikula, Jani <jani.nikula@intel.com>
>> > Subject: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in
>> > DSC disable
>> >
>> > There can be a chance that pre os has enabled DSC and driver's compute
>> > config would not need dsc to be enabled, in such case if we check on
>> > compute config's compression state to disable, we might end up in state
>> mismatch.
>> 
>> I assume this fixes real gitlab issue too?
> Okay, will add the tag 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3537

See https://lore.kernel.org/r/87fsxzp9qx.fsf@intel.com

The problem is with ->bigjoiner, not the entire statement.


BR,
Jani.

>
> Thanks,
> Vandita
>> 
>> >
>> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_vdsc.c | 4 ----
>> >  1 file changed, 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
>> > b/drivers/gpu/drm/i915/display/intel_vdsc.c
>> > index 19cd9531c115..b05a96011d93 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
>> > @@ -1161,10 +1161,6 @@ void intel_dsc_disable(const struct
>> > intel_crtc_state
>> > *old_crtc_state)
>> >  	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc);
>> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> >
>> > -	if (!(old_crtc_state->dsc.compression_enable &&
>> > -	      old_crtc_state->bigjoiner))
>> > -		return;
>> > -
>> >  	intel_de_write(dev_priv, dss_ctl1_reg(old_crtc_state), 0);
>> >  	intel_de_write(dev_priv, dss_ctl2_reg(old_crtc_state), 0);  }
>> > --
>> > 2.21.0.5.gaeb582a
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in DSC disable

[Kulkarni, Vandita]

> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Thursday, June 3, 2021 3:11 PM
> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; Saarinen, Jani
> <jani.saarinen@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Manna, Animesh <animesh.manna@intel.com>; Navare, Manasi D
> <manasi.d.navare@intel.com>
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in
> DSC disable
> 
> On Thu, 03 Jun 2021, "Kulkarni, Vandita" <vandita.kulkarni@intel.com>
> wrote:
> >> -----Original Message-----
> >> From: Saarinen, Jani <jani.saarinen@intel.com>
> >> Sent: Thursday, June 3, 2021 1:07 PM
> >> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
> >> gfx@lists.freedesktop.org
> >> Cc: Nikula, Jani <jani.nikula@intel.com>
> >> Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant
> >> checks in DSC disable
> >>
> >> Hi,
> >> > -----Original Message-----
> >> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
> >> > Of Vandita Kulkarni
> >> > Sent: torstai 3. kesäkuuta 2021 9.54
> >> > To: intel-gfx@lists.freedesktop.org
> >> > Cc: Nikula, Jani <jani.nikula@intel.com>
> >> > Subject: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks
> >> > in DSC disable
> >> >
> >> > There can be a chance that pre os has enabled DSC and driver's
> >> > compute config would not need dsc to be enabled, in such case if we
> >> > check on compute config's compression state to disable, we might
> >> > end up in state
> >> mismatch.
> >>
> >> I assume this fixes real gitlab issue too?
> > Okay, will add the tag
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3537
> 
> See https://lore.kernel.org/r/87fsxzp9qx.fsf@intel.com
> 
> The problem is with ->bigjoiner, not the entire statement.
Thanks for pointing this out, true that bigjoiner not being enabled will stop dsc disabling.
The bigjoiner check was making the entire condition check unnecessary.

Will update and refloat.

Thanks,
Vandita
> 
> 
> BR,
> Jani.
> 
> >
> > Thanks,
> > Vandita
> >>
> >> >
> >> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/display/intel_vdsc.c | 4 ----
> >> >  1 file changed, 4 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> >> > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> >> > index 19cd9531c115..b05a96011d93 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> >> > @@ -1161,10 +1161,6 @@ void intel_dsc_disable(const struct
> >> > intel_crtc_state
> >> > *old_crtc_state)
> >> >  	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc);
> >> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >> >
> >> > -	if (!(old_crtc_state->dsc.compression_enable &&
> >> > -	      old_crtc_state->bigjoiner))
> >> > -		return;
> >> > -
> >> >  	intel_de_write(dev_priv, dss_ctl1_reg(old_crtc_state), 0);
> >> >  	intel_de_write(dev_priv, dss_ctl2_reg(old_crtc_state), 0);  }
> >> > --
> >> > 2.21.0.5.gaeb582a
> >> >
> >> > _______________________________________________
> >> > Intel-gfx mailing list
> >> > Intel-gfx@lists.freedesktop.org
> >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in DSC disable

[Manna, Animesh]

> -----Original Message-----
> From: Kulkarni, Vandita <vandita.kulkarni@intel.com>
> Sent: Thursday, June 3, 2021 4:55 PM
> To: Nikula, Jani <jani.nikula@intel.com>; Saarinen, Jani
> <jani.saarinen@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Manna, Animesh <animesh.manna@intel.com>; Navare, Manasi D
> <manasi.d.navare@intel.com>
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in DSC
> disable
> 
> > -----Original Message-----
> > From: Nikula, Jani <jani.nikula@intel.com>
> > Sent: Thursday, June 3, 2021 3:11 PM
> > To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; Saarinen, Jani
> > <jani.saarinen@intel.com>; intel-gfx@lists.freedesktop.org
> > Cc: Manna, Animesh <animesh.manna@intel.com>; Navare, Manasi D
> > <manasi.d.navare@intel.com>
> > Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks
> > in DSC disable
> >
> > On Thu, 03 Jun 2021, "Kulkarni, Vandita" <vandita.kulkarni@intel.com>
> > wrote:
> > >> -----Original Message-----
> > >> From: Saarinen, Jani <jani.saarinen@intel.com>
> > >> Sent: Thursday, June 3, 2021 1:07 PM
> > >> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
> > >> gfx@lists.freedesktop.org
> > >> Cc: Nikula, Jani <jani.nikula@intel.com>
> > >> Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant
> > >> checks in DSC disable
> > >>
> > >> Hi,
> > >> > -----Original Message-----
> > >> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On
> > >> > Behalf Of Vandita Kulkarni
> > >> > Sent: torstai 3. kesäkuuta 2021 9.54
> > >> > To: intel-gfx@lists.freedesktop.org
> > >> > Cc: Nikula, Jani <jani.nikula@intel.com>
> > >> > Subject: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant
> > >> > checks in DSC disable
> > >> >
> > >> > There can be a chance that pre os has enabled DSC and driver's
> > >> > compute config would not need dsc to be enabled, in such case if
> > >> > we check on compute config's compression state to disable, we
> > >> > might end up in state
> > >> mismatch.
> > >>
> > >> I assume this fixes real gitlab issue too?
> > > Okay, will add the tag
> > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3537
> >
> > See https://lore.kernel.org/r/87fsxzp9qx.fsf@intel.com
> >
> > The problem is with ->bigjoiner, not the entire statement.
> Thanks for pointing this out, true that bigjoiner not being enabled will stop dsc
> disabling.
> The bigjoiner check was making the entire condition check unnecessary.
> 
> Will update and refloat.

Hi Jani/Vandita,

For uncompressed bigjoiner case if we want to use the same function to clear the dsc_ctrl1 register we may need to remove both the condition check.
As for uncompressed bigjoiner case, compression_enable Will be 0 and will block in clearing the dss_ctl1_reg.

Regards,
Animesh
> 
> Thanks,
> Vandita
> >
> >
> > BR,
> > Jani.
> >
> > >
> > > Thanks,
> > > Vandita
> > >>
> > >> >
> > >> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> > >> > ---
> > >> >  drivers/gpu/drm/i915/display/intel_vdsc.c | 4 ----
> > >> >  1 file changed, 4 deletions(-)
> > >> >
> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > >> > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > >> > index 19cd9531c115..b05a96011d93 100644
> > >> > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > >> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > >> > @@ -1161,10 +1161,6 @@ void intel_dsc_disable(const struct
> > >> > intel_crtc_state
> > >> > *old_crtc_state)
> > >> >  	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc);
> > >> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > >> >
> > >> > -	if (!(old_crtc_state->dsc.compression_enable &&
> > >> > -	      old_crtc_state->bigjoiner))
> > >> > -		return;
> > >> > -
> > >> >  	intel_de_write(dev_priv, dss_ctl1_reg(old_crtc_state), 0);
> > >> >  	intel_de_write(dev_priv, dss_ctl2_reg(old_crtc_state), 0);  }
> > >> > --
> > >> > 2.21.0.5.gaeb582a
> > >> >
> > >> > _______________________________________________
> > >> > Intel-gfx mailing list
> > >> > Intel-gfx@lists.freedesktop.org
> > >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in DSC disable

[Kulkarni, Vandita]

> -----Original Message-----
> From: Manna, Animesh <animesh.manna@intel.com>
> Sent: Thursday, June 3, 2021 7:24 PM
> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>; Saarinen, Jani <jani.saarinen@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Navare, Manasi D <manasi.d.navare@intel.com>
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in
> DSC disable
> 
> 
> 
> > -----Original Message-----
> > From: Kulkarni, Vandita <vandita.kulkarni@intel.com>
> > Sent: Thursday, June 3, 2021 4:55 PM
> > To: Nikula, Jani <jani.nikula@intel.com>; Saarinen, Jani
> > <jani.saarinen@intel.com>; intel-gfx@lists.freedesktop.org
> > Cc: Manna, Animesh <animesh.manna@intel.com>; Navare, Manasi D
> > <manasi.d.navare@intel.com>
> > Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks
> > in DSC disable
> >
> > > -----Original Message-----
> > > From: Nikula, Jani <jani.nikula@intel.com>
> > > Sent: Thursday, June 3, 2021 3:11 PM
> > > To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; Saarinen, Jani
> > > <jani.saarinen@intel.com>; intel-gfx@lists.freedesktop.org
> > > Cc: Manna, Animesh <animesh.manna@intel.com>; Navare, Manasi D
> > > <manasi.d.navare@intel.com>
> > > Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant
> > > checks in DSC disable
> > >
> > > On Thu, 03 Jun 2021, "Kulkarni, Vandita"
> > > <vandita.kulkarni@intel.com>
> > > wrote:
> > > >> -----Original Message-----
> > > >> From: Saarinen, Jani <jani.saarinen@intel.com>
> > > >> Sent: Thursday, June 3, 2021 1:07 PM
> > > >> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
> > > >> gfx@lists.freedesktop.org
> > > >> Cc: Nikula, Jani <jani.nikula@intel.com>
> > > >> Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant
> > > >> checks in DSC disable
> > > >>
> > > >> Hi,
> > > >> > -----Original Message-----
> > > >> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On
> > > >> > Behalf Of Vandita Kulkarni
> > > >> > Sent: torstai 3. kesäkuuta 2021 9.54
> > > >> > To: intel-gfx@lists.freedesktop.org
> > > >> > Cc: Nikula, Jani <jani.nikula@intel.com>
> > > >> > Subject: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant
> > > >> > checks in DSC disable
> > > >> >
> > > >> > There can be a chance that pre os has enabled DSC and driver's
> > > >> > compute config would not need dsc to be enabled, in such case
> > > >> > if we check on compute config's compression state to disable,
> > > >> > we might end up in state
> > > >> mismatch.
> > > >>
> > > >> I assume this fixes real gitlab issue too?
> > > > Okay, will add the tag
> > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3537
> > >
> > > See https://lore.kernel.org/r/87fsxzp9qx.fsf@intel.com
> > >
> > > The problem is with ->bigjoiner, not the entire statement.
> > Thanks for pointing this out, true that bigjoiner not being enabled
> > will stop dsc disabling.
> > The bigjoiner check was making the entire condition check unnecessary.
> >
> > Will update and refloat.
> 
> Hi Jani/Vandita,
> 
> For uncompressed bigjoiner case if we want to use the same function to
> clear the dsc_ctrl1 register we may need to remove both the condition
> check.
> As for uncompressed bigjoiner case, compression_enable Will be 0 and will
> block in clearing the dss_ctl1_reg.

Yes, I was going through and found that bit 20 and 21 of dss_ctl1 are being used
for uncompressed joiner.
So when dsc is not enabled to avoid writing the register we could add
below code .

if (dsc)
	clear dss_ctl2
if ( bigjoiner | dsc)
	clear dss_ctl1;
return;

bigjoiner = 1 and dsc = 0  - uncompressed , I think there is no harm in clearing dsc bits again
bigjoiner = 1 and dsc = 1 - compressed - uncompressed bits are already 0
bigjoiner = 0 and dsc= 1 - just dsc  - clear dsc rest are 0s already
bigjoiner = 0 and dsc = 0  do nothing, return

If I have missed any corner case, please let me know.

Thanks,
Vandita
> 
> Regards,
> Animesh
> >
> > Thanks,
> > Vandita
> > >
> > >
> > > BR,
> > > Jani.
> > >
> > > >
> > > > Thanks,
> > > > Vandita
> > > >>
> > > >> >
> > > >> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> > > >> > ---
> > > >> >  drivers/gpu/drm/i915/display/intel_vdsc.c | 4 ----
> > > >> >  1 file changed, 4 deletions(-)
> > > >> >
> > > >> > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > >> > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > >> > index 19cd9531c115..b05a96011d93 100644
> > > >> > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > >> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > >> > @@ -1161,10 +1161,6 @@ void intel_dsc_disable(const struct
> > > >> > intel_crtc_state
> > > >> > *old_crtc_state)
> > > >> >  	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state-
> >uapi.crtc);
> > > >> >  	struct drm_i915_private *dev_priv = to_i915(crtc-
> >base.dev);
> > > >> >
> > > >> > -	if (!(old_crtc_state->dsc.compression_enable &&
> > > >> > -	      old_crtc_state->bigjoiner))
> > > >> > -		return;
> > > >> > -
> > > >> >  	intel_de_write(dev_priv, dss_ctl1_reg(old_crtc_state), 0);
> > > >> >  	intel_de_write(dev_priv, dss_ctl2_reg(old_crtc_state), 0);  }
> > > >> > --
> > > >> > 2.21.0.5.gaeb582a
> > > >> >
> > > >> > _______________________________________________
> > > >> > Intel-gfx mailing list
> > > >> > Intel-gfx@lists.freedesktop.org
> > > >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >
> > > --
> > > Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in DSC disable

[Navare, Manasi]

On Thu, Jun 03, 2021 at 08:37:22AM -0700, Kulkarni, Vandita wrote:
> > -----Original Message-----
> > From: Manna, Animesh <animesh.manna@intel.com>
> > Sent: Thursday, June 3, 2021 7:24 PM
> > To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; Nikula, Jani
> > <jani.nikula@intel.com>; Saarinen, Jani <jani.saarinen@intel.com>; intel-
> > gfx@lists.freedesktop.org
> > Cc: Navare, Manasi D <manasi.d.navare@intel.com>
> > Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in
> > DSC disable
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: Kulkarni, Vandita <vandita.kulkarni@intel.com>
> > > Sent: Thursday, June 3, 2021 4:55 PM
> > > To: Nikula, Jani <jani.nikula@intel.com>; Saarinen, Jani
> > > <jani.saarinen@intel.com>; intel-gfx@lists.freedesktop.org
> > > Cc: Manna, Animesh <animesh.manna@intel.com>; Navare, Manasi D
> > > <manasi.d.navare@intel.com>
> > > Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks
> > > in DSC disable
> > >
> > > > -----Original Message-----
> > > > From: Nikula, Jani <jani.nikula@intel.com>
> > > > Sent: Thursday, June 3, 2021 3:11 PM
> > > > To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; Saarinen, Jani
> > > > <jani.saarinen@intel.com>; intel-gfx@lists.freedesktop.org
> > > > Cc: Manna, Animesh <animesh.manna@intel.com>; Navare, Manasi D
> > > > <manasi.d.navare@intel.com>
> > > > Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant
> > > > checks in DSC disable
> > > >
> > > > On Thu, 03 Jun 2021, "Kulkarni, Vandita"
> > > > <vandita.kulkarni@intel.com>
> > > > wrote:
> > > > >> -----Original Message-----
> > > > >> From: Saarinen, Jani <jani.saarinen@intel.com>
> > > > >> Sent: Thursday, June 3, 2021 1:07 PM
> > > > >> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
> > > > >> gfx@lists.freedesktop.org
> > > > >> Cc: Nikula, Jani <jani.nikula@intel.com>
> > > > >> Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant
> > > > >> checks in DSC disable
> > > > >>
> > > > >> Hi,
> > > > >> > -----Original Message-----
> > > > >> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On
> > > > >> > Behalf Of Vandita Kulkarni
> > > > >> > Sent: torstai 3. kesäkuuta 2021 9.54
> > > > >> > To: intel-gfx@lists.freedesktop.org
> > > > >> > Cc: Nikula, Jani <jani.nikula@intel.com>
> > > > >> > Subject: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant
> > > > >> > checks in DSC disable
> > > > >> >
> > > > >> > There can be a chance that pre os has enabled DSC and driver's
> > > > >> > compute config would not need dsc to be enabled, in such case
> > > > >> > if we check on compute config's compression state to disable,
> > > > >> > we might end up in state
> > > > >> mismatch.
> > > > >>
> > > > >> I assume this fixes real gitlab issue too?
> > > > > Okay, will add the tag
> > > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3537
> > > >
> > > > See https://lore.kernel.org/r/87fsxzp9qx.fsf@intel.com
> > > >
> > > > The problem is with ->bigjoiner, not the entire statement.
> > > Thanks for pointing this out, true that bigjoiner not being enabled
> > > will stop dsc disabling.
> > > The bigjoiner check was making the entire condition check unnecessary.
> > >
> > > Will update and refloat.
> > 
> > Hi Jani/Vandita,
> > 
> > For uncompressed bigjoiner case if we want to use the same function to
> > clear the dsc_ctrl1 register we may need to remove both the condition
> > check.
> > As for uncompressed bigjoiner case, compression_enable Will be 0 and will
> > block in clearing the dss_ctl1_reg.
> 
> Yes, I was going through and found that bit 20 and 21 of dss_ctl1 are being used
> for uncompressed joiner.
> So when dsc is not enabled to avoid writing the register we could add
> below code .
> 
> if (dsc)
> 	clear dss_ctl2
> if ( bigjoiner | dsc)
> 	clear dss_ctl1;
> return;
> 
> bigjoiner = 1 and dsc = 0  - uncompressed , I think there is no harm in clearing dsc bits again
> bigjoiner = 1 and dsc = 1 - compressed - uncompressed bits are already 0
> bigjoiner = 0 and dsc= 1 - just dsc  - clear dsc rest are 0s already
> bigjoiner = 0 and dsc = 0  do nothing, return
> 
> If I have missed any corner case, please let me know.
> 
> Thanks,
> Vandita

I think in the original code the condition was just reversed, instead it should be  :

if !(dsc_en || bigjoiner_en) {
	write 0 to dss ctl 1
	write 0 to dss ctl 2
}

So here basically it meets all the conditions you mentioned Vandita:

- only when both dsc and bigjoiner are 0, it will do nothing
- In all other cases DSC + Bigjoiner : Clear all bits including uncompressed bits which shd be 0 already
- In dsc = 0, bigjoiner = 1 (uncompressed), it will clear both again which is okay since dsc bits are already 0

Does this make sense?

Regards
Manasi


> > 
> > Regards,
> > Animesh
> > >
> > > Thanks,
> > > Vandita
> > > >
> > > >
> > > > BR,
> > > > Jani.
> > > >
> > > > >
> > > > > Thanks,
> > > > > Vandita
> > > > >>
> > > > >> >
> > > > >> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> > > > >> > ---
> > > > >> >  drivers/gpu/drm/i915/display/intel_vdsc.c | 4 ----
> > > > >> >  1 file changed, 4 deletions(-)
> > > > >> >
> > > > >> > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > > >> > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > > >> > index 19cd9531c115..b05a96011d93 100644
> > > > >> > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > > >> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > > >> > @@ -1161,10 +1161,6 @@ void intel_dsc_disable(const struct
> > > > >> > intel_crtc_state
> > > > >> > *old_crtc_state)
> > > > >> >  	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state-
> > >uapi.crtc);
> > > > >> >  	struct drm_i915_private *dev_priv = to_i915(crtc-
> > >base.dev);
> > > > >> >
> > > > >> > -	if (!(old_crtc_state->dsc.compression_enable &&
> > > > >> > -	      old_crtc_state->bigjoiner))
> > > > >> > -		return;
> > > > >> > -
> > > > >> >  	intel_de_write(dev_priv, dss_ctl1_reg(old_crtc_state), 0);
> > > > >> >  	intel_de_write(dev_priv, dss_ctl2_reg(old_crtc_state), 0);  }
> > > > >> > --
> > > > >> > 2.21.0.5.gaeb582a
> > > > >> >
> > > > >> > _______________________________________________
> > > > >> > Intel-gfx mailing list
> > > > >> > Intel-gfx@lists.freedesktop.org
> > > > >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > >
> > > > --
> > > > Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in DSC disable

[Kulkarni, Vandita]

> -----Original Message-----
> From: Navare, Manasi D <manasi.d.navare@intel.com>
> Sent: Friday, June 4, 2021 12:16 AM
> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>
> Cc: Manna, Animesh <animesh.manna@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>; Saarinen, Jani <jani.saarinen@intel.com>; intel-
> gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant checks in
> DSC disable
> 
> On Thu, Jun 03, 2021 at 08:37:22AM -0700, Kulkarni, Vandita wrote:
> > > -----Original Message-----
> > > From: Manna, Animesh <animesh.manna@intel.com>
> > > Sent: Thursday, June 3, 2021 7:24 PM
> > > To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; Nikula, Jani
> > > <jani.nikula@intel.com>; Saarinen, Jani <jani.saarinen@intel.com>;
> > > intel- gfx@lists.freedesktop.org
> > > Cc: Navare, Manasi D <manasi.d.navare@intel.com>
> > > Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant
> > > checks in DSC disable
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Kulkarni, Vandita <vandita.kulkarni@intel.com>
> > > > Sent: Thursday, June 3, 2021 4:55 PM
> > > > To: Nikula, Jani <jani.nikula@intel.com>; Saarinen, Jani
> > > > <jani.saarinen@intel.com>; intel-gfx@lists.freedesktop.org
> > > > Cc: Manna, Animesh <animesh.manna@intel.com>; Navare, Manasi D
> > > > <manasi.d.navare@intel.com>
> > > > Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant
> > > > checks in DSC disable
> > > >
> > > > > -----Original Message-----
> > > > > From: Nikula, Jani <jani.nikula@intel.com>
> > > > > Sent: Thursday, June 3, 2021 3:11 PM
> > > > > To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; Saarinen,
> > > > > Jani <jani.saarinen@intel.com>; intel-gfx@lists.freedesktop.org
> > > > > Cc: Manna, Animesh <animesh.manna@intel.com>; Navare, Manasi D
> > > > > <manasi.d.navare@intel.com>
> > > > > Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant
> > > > > checks in DSC disable
> > > > >
> > > > > On Thu, 03 Jun 2021, "Kulkarni, Vandita"
> > > > > <vandita.kulkarni@intel.com>
> > > > > wrote:
> > > > > >> -----Original Message-----
> > > > > >> From: Saarinen, Jani <jani.saarinen@intel.com>
> > > > > >> Sent: Thursday, June 3, 2021 1:07 PM
> > > > > >> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
> > > > > >> gfx@lists.freedesktop.org
> > > > > >> Cc: Nikula, Jani <jani.nikula@intel.com>
> > > > > >> Subject: RE: [Intel-gfx] [PATCH] drm/i915/dsc: Remove
> > > > > >> redundant checks in DSC disable
> > > > > >>
> > > > > >> Hi,
> > > > > >> > -----Original Message-----
> > > > > >> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org>
> > > > > >> > On Behalf Of Vandita Kulkarni
> > > > > >> > Sent: torstai 3. kesäkuuta 2021 9.54
> > > > > >> > To: intel-gfx@lists.freedesktop.org
> > > > > >> > Cc: Nikula, Jani <jani.nikula@intel.com>
> > > > > >> > Subject: [Intel-gfx] [PATCH] drm/i915/dsc: Remove redundant
> > > > > >> > checks in DSC disable
> > > > > >> >
> > > > > >> > There can be a chance that pre os has enabled DSC and
> > > > > >> > driver's compute config would not need dsc to be enabled,
> > > > > >> > in such case if we check on compute config's compression
> > > > > >> > state to disable, we might end up in state
> > > > > >> mismatch.
> > > > > >>
> > > > > >> I assume this fixes real gitlab issue too?
> > > > > > Okay, will add the tag
> > > > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3537
> > > > >
> > > > > See https://lore.kernel.org/r/87fsxzp9qx.fsf@intel.com
> > > > >
> > > > > The problem is with ->bigjoiner, not the entire statement.
> > > > Thanks for pointing this out, true that bigjoiner not being
> > > > enabled will stop dsc disabling.
> > > > The bigjoiner check was making the entire condition check
> unnecessary.
> > > >
> > > > Will update and refloat.
> > >
> > > Hi Jani/Vandita,
> > >
> > > For uncompressed bigjoiner case if we want to use the same function
> > > to clear the dsc_ctrl1 register we may need to remove both the
> > > condition check.
> > > As for uncompressed bigjoiner case, compression_enable Will be 0 and
> > > will block in clearing the dss_ctl1_reg.
> >
> > Yes, I was going through and found that bit 20 and 21 of dss_ctl1 are
> > being used for uncompressed joiner.
> > So when dsc is not enabled to avoid writing the register we could add
> > below code .
> >
> > if (dsc)
> > 	clear dss_ctl2
> > if ( bigjoiner | dsc)
> > 	clear dss_ctl1;
> > return;
> >
> > bigjoiner = 1 and dsc = 0  - uncompressed , I think there is no harm
> > in clearing dsc bits again bigjoiner = 1 and dsc = 1 - compressed -
> > uncompressed bits are already 0 bigjoiner = 0 and dsc= 1 - just dsc  -
> > clear dsc rest are 0s already bigjoiner = 0 and dsc = 0  do nothing,
> > return
> >
> > If I have missed any corner case, please let me know.
> >
> > Thanks,
> > Vandita
> 
> I think in the original code the condition was just reversed, instead it should
> be  :
> 
> if !(dsc_en || bigjoiner_en) {
> 	write 0 to dss ctl 1
> 	write 0 to dss ctl 2
> }
It should be the other way.
If neither of them are enabled then we have to just return.

Since I see that dss_ctl2 also gets set in case of big joiner I shall move that under
the main check of bigjoiner OR dsc and clear both the regs.

Thanks,
Vandita
> 
> So here basically it meets all the conditions you mentioned Vandita:
> 
> - only when both dsc and bigjoiner are 0, it will do nothing
> - In all other cases DSC + Bigjoiner : Clear all bits including uncompressed bits
> which shd be 0 already
> - In dsc = 0, bigjoiner = 1 (uncompressed), it will clear both again which is okay
> since dsc bits are already 0
> 
> Does this make sense?
> 
> Regards
> Manasi
> 
> 
> > >
> > > Regards,
> > > Animesh
> > > >
> > > > Thanks,
> > > > Vandita
> > > > >
> > > > >
> > > > > BR,
> > > > > Jani.
> > > > >
> > > > > >
> > > > > > Thanks,
> > > > > > Vandita
> > > > > >>
> > > > > >> >
> > > > > >> > Signed-off-by: Vandita Kulkarni
> > > > > >> > <vandita.kulkarni@intel.com>
> > > > > >> > ---
> > > > > >> >  drivers/gpu/drm/i915/display/intel_vdsc.c | 4 ----
> > > > > >> >  1 file changed, 4 deletions(-)
> > > > > >> >
> > > > > >> > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > > > >> > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > > > >> > index 19cd9531c115..b05a96011d93 100644
> > > > > >> > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > > > >> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > > > >> > @@ -1161,10 +1161,6 @@ void intel_dsc_disable(const struct
> > > > > >> > intel_crtc_state
> > > > > >> > *old_crtc_state)
> > > > > >> >  	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state-
> > > >uapi.crtc);
> > > > > >> >  	struct drm_i915_private *dev_priv = to_i915(crtc-
> > > >base.dev);
> > > > > >> >
> > > > > >> > -	if (!(old_crtc_state->dsc.compression_enable &&
> > > > > >> > -	      old_crtc_state->bigjoiner))
> > > > > >> > -		return;
> > > > > >> > -
> > > > > >> >  	intel_de_write(dev_priv, dss_ctl1_reg(old_crtc_state), 0);
> > > > > >> >  	intel_de_write(dev_priv, dss_ctl2_reg(old_crtc_state),
> > > > > >> > 0);  }
> > > > > >> > --
> > > > > >> > 2.21.0.5.gaeb582a
> > > > > >> >
> > > > > >> > _______________________________________________
> > > > > >> > Intel-gfx mailing list
> > > > > >> > Intel-gfx@lists.freedesktop.org
> > > > > >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > >
> > > > > --
> > > > > Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx