diff mbox series

[1/2] drm/i915/display/vlv: fix pixel overlap register update

Message ID 20221219092428.2515430-1-andrzej.hajda@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/i915/display/vlv: fix pixel overlap register update | expand

Commit Message

Andrzej Hajda Dec. 19, 2022, 9:24 a.m. UTC
To update properly bits in the register the mask should be used
to clear old value and then the result should be or-ed with new
value, for such updates there is separate helper intel_de_rmw.

Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
 drivers/gpu/drm/i915/display/vlv_dsi.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

Comments

Jani Nikula Dec. 19, 2022, 9:35 a.m. UTC | #1
On Mon, 19 Dec 2022, Andrzej Hajda <andrzej.hajda@intel.com> wrote:
> To update properly bits in the register the mask should be used
> to clear old value and then the result should be or-ed with new
> value, for such updates there is separate helper intel_de_rmw.
>
> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>

Seems like the right thing to do.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/display/vlv_dsi.c | 24 +++++++++---------------
>  1 file changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 662bdb656aa304..f5268997a3e172 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -649,23 +649,17 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder,
>  	enum port port;
>  
>  	if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
> -		u32 temp;
> +		u32 temp = intel_dsi->pixel_overlap;
> +
>  		if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv)) {
> -			for_each_dsi_port(port, intel_dsi->ports) {
> -				temp = intel_de_read(dev_priv,
> -						     MIPI_CTRL(port));
> -				temp &= ~BXT_PIXEL_OVERLAP_CNT_MASK |
> -					intel_dsi->pixel_overlap <<
> -					BXT_PIXEL_OVERLAP_CNT_SHIFT;
> -				intel_de_write(dev_priv, MIPI_CTRL(port),
> -					       temp);
> -			}
> +			for_each_dsi_port(port, intel_dsi->ports)
> +				intel_de_rmw(dev_priv, MIPI_CTRL(port),
> +					     BXT_PIXEL_OVERLAP_CNT_MASK,
> +					     temp << BXT_PIXEL_OVERLAP_CNT_SHIFT);
>  		} else {
> -			temp = intel_de_read(dev_priv, VLV_CHICKEN_3);
> -			temp &= ~PIXEL_OVERLAP_CNT_MASK |
> -					intel_dsi->pixel_overlap <<
> -					PIXEL_OVERLAP_CNT_SHIFT;
> -			intel_de_write(dev_priv, VLV_CHICKEN_3, temp);
> +			intel_de_rmw(dev_priv, VLV_CHICKEN_3,
> +				     PIXEL_OVERLAP_CNT_MASK,
> +				     temp << PIXEL_OVERLAP_CNT_SHIFT);
>  		}
>  	}
Andrzej Hajda Dec. 19, 2022, 3:35 p.m. UTC | #2
On 19.12.2022 15:41, Patchwork wrote:
> Project List - Patchwork *Patch Details*
> *Series:* 	series starting with [1/2] drm/i915/display/vlv: fix pixel 
> overlap register update
> *URL:* 	https://patchwork.freedesktop.org/series/112045/
> *State:* 	failure
> *Details:* 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/index.html
>
>
>   CI Bug Log - changes from CI_DRM_12515 -> Patchwork_112045v1
>
>
>     Summary
>
> *FAILURE*
>
> Serious unknown changes coming with Patchwork_112045v1 absolutely need 
> to be
> verified manually.
>
> If you think the reported changes have nothing to do with the changes
> introduced in Patchwork_112045v1, 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_112045v1/index.html
>
>
>     Participating hosts (40 -> 41)
>
> Additional (2): fi-snb-2520m bat-adlp-4
> Missing (1): fi-icl-u2
>
>
>     Possible new issues
>
> Here are the unknown changes that may have been introduced in 
> Patchwork_112045v1:
>
>
>       CI changes
>
>
>         Possible regressions
>
>   * boot:
>       o fi-ivb-3770: PASS
>         <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12515/fi-ivb-3770/boot.html>
>         -> FAIL
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/fi-ivb-3770/boot.html>
>

Apparently some CI failure, boot log looks OK, in run0.txt I've spotted line
     Build step 'Execute shell' marked build as failure
Probably some CI issue, queued re-test to be sure.

[1]: 
http://gfx-ci.igk.intel.com/tree/drm-tip/Patchwork_112045v1/fi-ivb-3770/run0.txt

Regards
Andrzej

>  *
>
>
>       IGT changes
>
>
>         Suppressed
>
> The following results come from untrusted machines, tests, or statuses.
> They do not affect the overall result.
>
>   * igt@i915_selftest@live@workarounds:
>       o {bat-rplp-1}: PASS
>         <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12515/bat-rplp-1/igt@i915_selftest@live@workarounds.html>
>         -> INCOMPLETE
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/bat-rplp-1/igt@i915_selftest@live@workarounds.html>
>
>
>     Known issues
>
> Here are the changes found in Patchwork_112045v1 that come from known 
> issues:
>
>
>       IGT changes
>
>
>         Issues hit
>
>  *
>
>     igt@debugfs_test@basic-hwmon:
>
>       o bat-adlp-4: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/bat-adlp-4/igt@debugfs_test@basic-hwmon.html>
>         (i915#7456 <https://gitlab.freedesktop.org/drm/intel/issues/7456>)
>  *
>
>     igt@gem_exec_gttfill@basic:
>
>       o fi-pnv-d510: PASS
>         <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12515/fi-pnv-d510/igt@gem_exec_gttfill@basic.html>
>         -> FAIL
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/fi-pnv-d510/igt@gem_exec_gttfill@basic.html>
>         (i915#7229 <https://gitlab.freedesktop.org/drm/intel/issues/7229>)
>  *
>
>     igt@gem_lmem_swapping@parallel-random-engines:
>
>       o bat-adlp-4: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/bat-adlp-4/igt@gem_lmem_swapping@parallel-random-engines.html>
>         (i915#4613
>         <https://gitlab.freedesktop.org/drm/intel/issues/4613>) +3
>         similar issues
>  *
>
>     igt@gem_tiled_pread_basic:
>
>       o bat-adlp-4: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/bat-adlp-4/igt@gem_tiled_pread_basic.html>
>         (i915#3282 <https://gitlab.freedesktop.org/drm/intel/issues/3282>)
>  *
>
>     igt@i915_pm_rps@basic-api:
>
>       o bat-adlp-4: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/bat-adlp-4/igt@i915_pm_rps@basic-api.html>
>         (i915#6621 <https://gitlab.freedesktop.org/drm/intel/issues/6621>)
>  *
>
>     igt@i915_selftest@live@late_gt_pm:
>
>       o fi-kbl-soraka: PASS
>         <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12515/fi-kbl-soraka/igt@i915_selftest@live@late_gt_pm.html>
>         -> INCOMPLETE
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/fi-kbl-soraka/igt@i915_selftest@live@late_gt_pm.html>
>         (i915#7640 <https://gitlab.freedesktop.org/drm/intel/issues/7640>)
>  *
>
>     igt@kms_chamelium@common-hpd-after-suspend:
>
>       o fi-hsw-4770: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/fi-hsw-4770/igt@kms_chamelium@common-hpd-after-suspend.html>
>         (fdo#109271
>         <https://bugs.freedesktop.org/show_bug.cgi?id=109271> /
>         fdo#111827 <https://bugs.freedesktop.org/show_bug.cgi?id=111827>)
>  *
>
>     igt@kms_chamelium@dp-crc-fast:
>
>       o bat-adlp-4: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/bat-adlp-4/igt@kms_chamelium@dp-crc-fast.html>
>         (fdo#111827
>         <https://bugs.freedesktop.org/show_bug.cgi?id=111827>) +8
>         similar issues
>  *
>
>     igt@kms_chamelium@hdmi-crc-fast:
>
>       o fi-snb-2520m: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/fi-snb-2520m/igt@kms_chamelium@hdmi-crc-fast.html>
>         (fdo#109271
>         <https://bugs.freedesktop.org/show_bug.cgi?id=109271> /
>         fdo#111827
>         <https://bugs.freedesktop.org/show_bug.cgi?id=111827>) +8
>         similar issues
>  *
>
>     igt@kms_cursor_legacy@basic-busy-flip-before-cursor:
>
>       o bat-adlp-4: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/bat-adlp-4/igt@kms_cursor_legacy@basic-busy-flip-before-cursor.html>
>         (i915#4103 <https://gitlab.freedesktop.org/drm/intel/issues/4103>)
>  *
>
>     igt@kms_force_connector_basic@force-load-detect:
>
>       o bat-adlp-4: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/bat-adlp-4/igt@kms_force_connector_basic@force-load-detect.html>
>         (i915#4093
>         <https://gitlab.freedesktop.org/drm/intel/issues/4093>) +3
>         similar issues
>  *
>
>     igt@kms_setmode@basic-clone-single-crtc:
>
>       o bat-adlp-4: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/bat-adlp-4/igt@kms_setmode@basic-clone-single-crtc.html>
>         (i915#3555
>         <https://gitlab.freedesktop.org/drm/intel/issues/3555> /
>         i915#4579 <https://gitlab.freedesktop.org/drm/intel/issues/4579>)
>  *
>
>     igt@prime_vgem@basic-fence-flip:
>
>       o fi-snb-2520m: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/fi-snb-2520m/igt@prime_vgem@basic-fence-flip.html>
>         (fdo#109271
>         <https://bugs.freedesktop.org/show_bug.cgi?id=109271>) +23
>         similar issues
>  *
>
>     igt@prime_vgem@basic-userptr:
>
>       o bat-adlp-4: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/bat-adlp-4/igt@prime_vgem@basic-userptr.html>
>         (fdo#109295
>         <https://bugs.freedesktop.org/show_bug.cgi?id=109295> /
>         i915#3301
>         <https://gitlab.freedesktop.org/drm/intel/issues/3301> /
>         i915#3708 <https://gitlab.freedesktop.org/drm/intel/issues/3708>)
>  *
>
>     igt@prime_vgem@basic-write:
>
>       o bat-adlp-4: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/bat-adlp-4/igt@prime_vgem@basic-write.html>
>         (fdo#109295
>         <https://bugs.freedesktop.org/show_bug.cgi?id=109295> /
>         i915#3291
>         <https://gitlab.freedesktop.org/drm/intel/issues/3291> /
>         i915#3708
>         <https://gitlab.freedesktop.org/drm/intel/issues/3708>) +2
>         similar issues
>
>
>         Possible fixes
>
>  *
>
>     igt@i915_selftest@live@hangcheck:
>
>      o
>
>         fi-hsw-4770: INCOMPLETE
>         <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12515/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html>
>         (i915#4785
>         <https://gitlab.freedesktop.org/drm/intel/issues/4785>) ->
>         PASS
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html>
>
>      o
>
>         {bat-adlm-1}: INCOMPLETE
>         <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12515/bat-adlm-1/igt@i915_selftest@live@hangcheck.html>
>         (i915#4983
>         <https://gitlab.freedesktop.org/drm/intel/issues/4983>) ->
>         PASS
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/bat-adlm-1/igt@i915_selftest@live@hangcheck.html>
>
>  *
>
>     igt@i915_selftest@live@workarounds:
>
>       o {bat-dg1-7}: DMESG-FAIL
>         <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12515/bat-dg1-7/igt@i915_selftest@live@workarounds.html>
>         -> PASS
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/bat-dg1-7/igt@i915_selftest@live@workarounds.html>
>  *
>
>     igt@i915_suspend@basic-s3-without-i915:
>
>       o {bat-adln-1}: DMESG-WARN
>         <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12515/bat-adln-1/igt@i915_suspend@basic-s3-without-i915.html>
>         (i915#2867
>         <https://gitlab.freedesktop.org/drm/intel/issues/2867>) ->
>         PASS
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/bat-adln-1/igt@i915_suspend@basic-s3-without-i915.html>
>  *
>
>     igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-edp-1:
>
>       o {bat-adlp-6}: DMESG-WARN
>         <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12515/bat-adlp-6/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-edp-1.html>
>         (i915#2867
>         <https://gitlab.freedesktop.org/drm/intel/issues/2867>) ->
>         PASS
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112045v1/bat-adlp-6/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-edp-1.html>
>
> {name}: This element is suppressed. This means it is ignored when 
> computing
> the status of the difference (SUCCESS, WARNING, or FAILURE).
>
>
>     Build changes
>
>   * Linux: CI_DRM_12515 -> Patchwork_112045v1
>
> CI-20190529: 20190529
> CI_DRM_12515: 25905f33e9cb1630d94ce04688d5a6c756f96e9d @ 
> git://anongit.freedesktop.org/gfx-ci/linux
> IGT_7098: d24d605d1b94d404e5ccea13eed607b81e233696 @ 
> https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
> Patchwork_112045v1: 25905f33e9cb1630d94ce04688d5a6c756f96e9d @ 
> git://anongit.freedesktop.org/gfx-ci/linux
>
>
>       Linux commits
>
> e58edd2d7d00 drm/i915/display/vlv: use intel_de_rmw if possible
> 3f77a83c4a5c drm/i915/display/vlv: fix pixel overlap register update
>
Jani Nikula Jan. 30, 2023, 12:58 p.m. UTC | #3
On Mon, 19 Dec 2022, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Mon, 19 Dec 2022, Andrzej Hajda <andrzej.hajda@intel.com> wrote:
>> To update properly bits in the register the mask should be used
>> to clear old value and then the result should be or-ed with new
>> value, for such updates there is separate helper intel_de_rmw.
>>
>> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
>
> Seems like the right thing to do.
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

And pushed both to din, thanks for the patches.

BR,
Jani.


>
>
>> ---
>>  drivers/gpu/drm/i915/display/vlv_dsi.c | 24 +++++++++---------------
>>  1 file changed, 9 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
>> index 662bdb656aa304..f5268997a3e172 100644
>> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
>> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
>> @@ -649,23 +649,17 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder,
>>  	enum port port;
>>  
>>  	if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
>> -		u32 temp;
>> +		u32 temp = intel_dsi->pixel_overlap;
>> +
>>  		if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv)) {
>> -			for_each_dsi_port(port, intel_dsi->ports) {
>> -				temp = intel_de_read(dev_priv,
>> -						     MIPI_CTRL(port));
>> -				temp &= ~BXT_PIXEL_OVERLAP_CNT_MASK |
>> -					intel_dsi->pixel_overlap <<
>> -					BXT_PIXEL_OVERLAP_CNT_SHIFT;
>> -				intel_de_write(dev_priv, MIPI_CTRL(port),
>> -					       temp);
>> -			}
>> +			for_each_dsi_port(port, intel_dsi->ports)
>> +				intel_de_rmw(dev_priv, MIPI_CTRL(port),
>> +					     BXT_PIXEL_OVERLAP_CNT_MASK,
>> +					     temp << BXT_PIXEL_OVERLAP_CNT_SHIFT);
>>  		} else {
>> -			temp = intel_de_read(dev_priv, VLV_CHICKEN_3);
>> -			temp &= ~PIXEL_OVERLAP_CNT_MASK |
>> -					intel_dsi->pixel_overlap <<
>> -					PIXEL_OVERLAP_CNT_SHIFT;
>> -			intel_de_write(dev_priv, VLV_CHICKEN_3, temp);
>> +			intel_de_rmw(dev_priv, VLV_CHICKEN_3,
>> +				     PIXEL_OVERLAP_CNT_MASK,
>> +				     temp << PIXEL_OVERLAP_CNT_SHIFT);
>>  		}
>>  	}
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
index 662bdb656aa304..f5268997a3e172 100644
--- a/drivers/gpu/drm/i915/display/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
@@ -649,23 +649,17 @@  static void intel_dsi_port_enable(struct intel_encoder *encoder,
 	enum port port;
 
 	if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
-		u32 temp;
+		u32 temp = intel_dsi->pixel_overlap;
+
 		if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv)) {
-			for_each_dsi_port(port, intel_dsi->ports) {
-				temp = intel_de_read(dev_priv,
-						     MIPI_CTRL(port));
-				temp &= ~BXT_PIXEL_OVERLAP_CNT_MASK |
-					intel_dsi->pixel_overlap <<
-					BXT_PIXEL_OVERLAP_CNT_SHIFT;
-				intel_de_write(dev_priv, MIPI_CTRL(port),
-					       temp);
-			}
+			for_each_dsi_port(port, intel_dsi->ports)
+				intel_de_rmw(dev_priv, MIPI_CTRL(port),
+					     BXT_PIXEL_OVERLAP_CNT_MASK,
+					     temp << BXT_PIXEL_OVERLAP_CNT_SHIFT);
 		} else {
-			temp = intel_de_read(dev_priv, VLV_CHICKEN_3);
-			temp &= ~PIXEL_OVERLAP_CNT_MASK |
-					intel_dsi->pixel_overlap <<
-					PIXEL_OVERLAP_CNT_SHIFT;
-			intel_de_write(dev_priv, VLV_CHICKEN_3, temp);
+			intel_de_rmw(dev_priv, VLV_CHICKEN_3,
+				     PIXEL_OVERLAP_CNT_MASK,
+				     temp << PIXEL_OVERLAP_CNT_SHIFT);
 		}
 	}