[PATCH] drm/i915/dp: use logical operators with boolean type

[PATCH] drm/i915/dp: use logical operators with boolean type

[Jani Nikula]

Using arithmetic operators with booleans is confusing. Switch to logical
operators.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4e7b8d..ef4992f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5094,7 +5094,7 @@ static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
 	enum port port = intel_dig_port->base.port;
 	enum tc_port_type old_type = intel_dig_port->tc_type;
 
-	WARN_ON(is_legacy + is_typec + is_tbt != 1);
+	WARN_ON(is_legacy || is_typec || !is_tbt);
 
 	if (is_legacy)
 		intel_dig_port->tc_type = TC_PORT_LEGACY;
-- 
2.20.1

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

✗ Fi.CI.BAT: failure for drm/i915/dp: use logical operators with boolean type

[Patchwork]

== Series Details ==

Series: drm/i915/dp: use logical operators with boolean type
URL   : https://patchwork.freedesktop.org/series/60190/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6026 -> Patchwork_12935
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/60190/revisions/1/mbox/

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@runner@aborted:
    - fi-icl-u3:          NOTRUN -> [FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12935/fi-icl-u3/igt@runner@aborted.html

  
#### Warnings ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-apl-guc:         [DMESG-WARN][2] ([fdo#110512]) -> [FAIL][3]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6026/fi-apl-guc/igt@gem_exec_suspend@basic-s3.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12935/fi-apl-guc/igt@gem_exec_suspend@basic-s3.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-apl-guc:         [PASS][4] -> [DMESG-WARN][5] ([fdo#110512])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6026/fi-apl-guc/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12935/fi-apl-guc/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  
#### Possible fixes ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       [DMESG-WARN][6] ([fdo#108965]) -> [PASS][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6026/fi-kbl-8809g/igt@amdgpu/amd_basic@userptr.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12935/fi-kbl-8809g/igt@amdgpu/amd_basic@userptr.html

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       [INCOMPLETE][8] ([fdo#107718] / [fdo#110581]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6026/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12935/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@i915_selftest@live_hangcheck:
    - fi-skl-iommu:       [INCOMPLETE][10] ([fdo#108602] / [fdo#108744] / [fdo#110581]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6026/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12935/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html

  
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
  [fdo#110512]: https://bugs.freedesktop.org/show_bug.cgi?id=110512
  [fdo#110581]: https://bugs.freedesktop.org/show_bug.cgi?id=110581


Participating hosts (52 -> 46)
------------------------------

  Additional (1): fi-byt-j1900 
  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_6026 -> Patchwork_12935

  CI_DRM_6026: 2d2d8d3b9d8896c99c88307a881120885afd2ddb @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4972: f052e49a43cc9704ea5f240df15dd9d3dfed68ab @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12935: a0c6eb96eafc2476ab98276783fc1509c44e37b3 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a0c6eb96eafc drm/i915/dp: use logical operators with boolean type

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12935/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp: use logical operators with boolean type

[Paulo Zanoni]

Em qui, 2019-05-02 às 11:29 +0300, Jani Nikula escreveu:
> Using arithmetic operators with booleans is confusing. Switch to logical
> operators.
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 4e7b8d..ef4992f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5094,7 +5094,7 @@ static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
>  	enum port port = intel_dig_port->base.port;
>  	enum tc_port_type old_type = intel_dig_port->tc_type;
>  
> -	WARN_ON(is_legacy + is_typec + is_tbt != 1);
> +	WARN_ON(is_legacy || is_typec || !is_tbt);

This changes the meaning. You're interpreting this as:

    WARN_ON(is_legacy + is_typec + (is_tbt != 1))

while the original intent of the code is to be:

    WARN_ON((is_legacy + is_typec + is_tbt) != 1)

and a quick check on operator precedence tables leads me to think the
original code is indeed correct.

We're asserting exactly one of these bools enabled, so the logic
operation would be something like:

WARN_ON((is_legacy && (is_typec || is_tbt)) ||
        (is_typec && (is_legacy || is_tbt)) ||
 	(is_tbt && (is_legacy || is_typec)) ||
	(!is_legacy && !is_typec && !is_tbt))

I would still prefer the arithmetic operation.    

>  
>  	if (is_legacy)
>  		intel_dig_port->tc_type = TC_PORT_LEGACY;

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

✗ Fi.CI.BAT: failure for drm/i915/dp: use logical operators with boolean type (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/dp: use logical operators with boolean type (rev2)
URL   : https://patchwork.freedesktop.org/series/60190/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6029 -> Patchwork_12944
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/60190/revisions/2/mbox/

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@runner@aborted:
    - fi-icl-u3:          NOTRUN -> [FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12944/fi-icl-u3/igt@runner@aborted.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       [PASS][2] -> [INCOMPLETE][3] ([fdo#107718] / [fdo#110581])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6029/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12944/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@i915_selftest@live_evict:
    - fi-bsw-kefka:       [PASS][4] -> [DMESG-WARN][5] ([fdo#107709])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6029/fi-bsw-kefka/igt@i915_selftest@live_evict.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12944/fi-bsw-kefka/igt@i915_selftest@live_evict.html

  * igt@i915_selftest@live_hangcheck:
    - fi-icl-y:           [PASS][6] -> [INCOMPLETE][7] ([fdo#107713] / [fdo#108569] / [fdo#110581])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6029/fi-icl-y/igt@i915_selftest@live_hangcheck.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12944/fi-icl-y/igt@i915_selftest@live_hangcheck.html

  
#### Possible fixes ####

  * igt@i915_selftest@live_hangcheck:
    - fi-skl-iommu:       [INCOMPLETE][8] ([fdo#108602] / [fdo#108744] / [fdo#110581]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6029/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12944/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html

  
  [fdo#107709]: https://bugs.freedesktop.org/show_bug.cgi?id=107709
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#110581]: https://bugs.freedesktop.org/show_bug.cgi?id=110581


Participating hosts (54 -> 44)
------------------------------

  Missing    (10): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-skl-guc fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus fi-byt-clapper fi-skl-6600u 


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

  * Linux: CI_DRM_6029 -> Patchwork_12944

  CI_DRM_6029: 0548213ff6d52d4638778a95a4b3a7900e683ac3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4972: f052e49a43cc9704ea5f240df15dd9d3dfed68ab @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12944: f93b4413e5038b02f3af44a46ce4a5e188f2906e @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

f93b4413e503 drm/i915/dp: use logical operators with boolean type

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12944/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp: use logical operators with boolean type

[Jani Nikula]

On Thu, 02 May 2019, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> Em qui, 2019-05-02 às 11:29 +0300, Jani Nikula escreveu:
>> Using arithmetic operators with booleans is confusing. Switch to logical
>> operators.
>> 
>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 4e7b8d..ef4992f 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -5094,7 +5094,7 @@ static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
>>  	enum port port = intel_dig_port->base.port;
>>  	enum tc_port_type old_type = intel_dig_port->tc_type;
>>  
>> -	WARN_ON(is_legacy + is_typec + is_tbt != 1);
>> +	WARN_ON(is_legacy || is_typec || !is_tbt);
>
> This changes the meaning. You're interpreting this as:
>
>     WARN_ON(is_legacy + is_typec + (is_tbt != 1))
>
> while the original intent of the code is to be:
>
>     WARN_ON((is_legacy + is_typec + is_tbt) != 1)

*blush*

> and a quick check on operator precedence tables leads me to think the
> original code is indeed correct.
>
> We're asserting exactly one of these bools enabled, so the logic
> operation would be something like:
>
> WARN_ON((is_legacy && (is_typec || is_tbt)) ||
>         (is_typec && (is_legacy || is_tbt)) ||
>  	(is_tbt && (is_legacy || is_typec)) ||
> 	(!is_legacy && !is_typec && !is_tbt))
>
> I would still prefer the arithmetic operation.

Agreed.

I'll go hide under a rock.


BR,
Jani.

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