* [bug report] drm/bridge: synopsys: Add DW DPTX Controller support library
@ 2025-09-02 6:35 Dan Carpenter
2025-09-02 11:42 ` Andy Yan
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2025-09-02 6:35 UTC (permalink / raw)
To: Andy Yan; +Cc: dri-devel
Hello Andy Yan,
Commit 86eecc3a9c2e ("drm/bridge: synopsys: Add DW DPTX Controller
support library") from Aug 22, 2025 (linux-next), leads to the
following Smatch static checker warning:
drivers/gpu/drm/bridge/synopsys/dw-dp.c:730 dw_dp_voltage_max()
warn: bitwise AND is zero '0x3 & 0x18'
drivers/gpu/drm/bridge/synopsys/dw-dp.c
728 static u8 dw_dp_voltage_max(u8 preemph)
729 {
--> 730 switch (preemph & DP_TRAIN_PRE_EMPHASIS_MASK) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This mask will always be zero (DP_TRAIN_PRE_EMPH_LEVEL_0).
731 case DP_TRAIN_PRE_EMPH_LEVEL_0:
732 return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
733 case DP_TRAIN_PRE_EMPH_LEVEL_1:
734 return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
735 case DP_TRAIN_PRE_EMPH_LEVEL_2:
736 return DP_TRAIN_VOLTAGE_SWING_LEVEL_1;
737 case DP_TRAIN_PRE_EMPH_LEVEL_3:
738 default:
739 return DP_TRAIN_VOLTAGE_SWING_LEVEL_0;
740 }
741 }
The problem is the inconsistent >> 3 shifting. Here is how the
caller looks like:
755 p = drm_dp_get_adjust_request_pre_emphasis(status, i);
p is a 0x3 << 3 mask
756 p >>= DP_TRAIN_PRE_EMPHASIS_SHIFT;
We shift it >> 3 for convenience
757
758 if (v != adj->voltage_swing[i] || p != adj->pre_emphasis[i])
759 changed = true;
760
761 if (p >= (DP_TRAIN_PRE_EMPH_LEVEL_3 >> DP_TRAIN_PRE_EMPHASIS_SHIFT)) {
762 adj->pre_emphasis[i] = DP_TRAIN_PRE_EMPH_LEVEL_3 >>
763 DP_TRAIN_PRE_EMPHASIS_SHIFT;
764 adj->pre_max_reached[i] = true;
765 } else {
766 adj->pre_emphasis[i] = p;
767 adj->pre_max_reached[i] = false;
768 }
769
770 v = min(v, dw_dp_voltage_max(p));
^
But the dw_dp_voltage_max() function expects the unshifted value.
There is another similar warning but it's probably deliberate.
drivers/gpu/drm/bridge/synopsys/dw-dp.c:1072 dw_dp_send_sdp() warn: bitwise AND is zero '0x1 & 0x2'
drivers/gpu/drm/bridge/synopsys/dw-dp.c
1066
1067 if (sdp->flags & DW_DP_SDP_VERTICAL_INTERVAL)
1068 regmap_update_bits(dp->regmap, DW_DP_SDP_VERTICAL_CTRL,
1069 EN_VERTICAL_SDP << nr,
1070 EN_VERTICAL_SDP << nr);
1071
1072 if (sdp->flags & DW_DP_SDP_HORIZONTAL_INTERVAL)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
DW_DP_SDP_HORIZONTAL_INTERVAL is never used.
1073 regmap_update_bits(dp->regmap, DW_DP_SDP_HORIZONTAL_CTRL,
1074 EN_HORIZONTAL_SDP << nr,
1075 EN_HORIZONTAL_SDP << nr);
1076
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re:[bug report] drm/bridge: synopsys: Add DW DPTX Controller support library
2025-09-02 6:35 [bug report] drm/bridge: synopsys: Add DW DPTX Controller support library Dan Carpenter
@ 2025-09-02 11:42 ` Andy Yan
2025-09-02 11:45 ` [bug " Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Andy Yan @ 2025-09-02 11:42 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Andy Yan, dri-devel
Hello Dan,
See my reply inline.
At 2025-09-02 14:35:26, "Dan Carpenter" <dan.carpenter@linaro.org> wrote:
>Hello Andy Yan,
>
>Commit 86eecc3a9c2e ("drm/bridge: synopsys: Add DW DPTX Controller
>support library") from Aug 22, 2025 (linux-next), leads to the
>following Smatch static checker warning:
>
> drivers/gpu/drm/bridge/synopsys/dw-dp.c:730 dw_dp_voltage_max()
> warn: bitwise AND is zero '0x3 & 0x18'
Thanks for catching this, I sent a patch try to fix this issue here[0]:
>
>drivers/gpu/drm/bridge/synopsys/dw-dp.c
> 728 static u8 dw_dp_voltage_max(u8 preemph)
> 729 {
>--> 730 switch (preemph & DP_TRAIN_PRE_EMPHASIS_MASK) {
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>This mask will always be zero (DP_TRAIN_PRE_EMPH_LEVEL_0).
>
> 731 case DP_TRAIN_PRE_EMPH_LEVEL_0:
> 732 return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> 733 case DP_TRAIN_PRE_EMPH_LEVEL_1:
> 734 return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> 735 case DP_TRAIN_PRE_EMPH_LEVEL_2:
> 736 return DP_TRAIN_VOLTAGE_SWING_LEVEL_1;
> 737 case DP_TRAIN_PRE_EMPH_LEVEL_3:
> 738 default:
> 739 return DP_TRAIN_VOLTAGE_SWING_LEVEL_0;
> 740 }
> 741 }
>
>The problem is the inconsistent >> 3 shifting. Here is how the
>caller looks like:
>
> 755 p = drm_dp_get_adjust_request_pre_emphasis(status, i);
>
>p is a 0x3 << 3 mask
>
> 756 p >>= DP_TRAIN_PRE_EMPHASIS_SHIFT;
>
>We shift it >> 3 for convenience
>
> 757
> 758 if (v != adj->voltage_swing[i] || p != adj->pre_emphasis[i])
> 759 changed = true;
> 760
> 761 if (p >= (DP_TRAIN_PRE_EMPH_LEVEL_3 >> DP_TRAIN_PRE_EMPHASIS_SHIFT)) {
> 762 adj->pre_emphasis[i] = DP_TRAIN_PRE_EMPH_LEVEL_3 >>
> 763 DP_TRAIN_PRE_EMPHASIS_SHIFT;
> 764 adj->pre_max_reached[i] = true;
> 765 } else {
> 766 adj->pre_emphasis[i] = p;
> 767 adj->pre_max_reached[i] = false;
> 768 }
> 769
> 770 v = min(v, dw_dp_voltage_max(p));
> ^
>But the dw_dp_voltage_max() function expects the unshifted value.
>
>There is another similar warning but it's probably deliberate.
Yes, this is deliberate, we will set DW_DP_SDP_HORIZONTAL_INTERVAL when we add support for audio in the
future. So I think we can let it as it is now.
>drivers/gpu/drm/bridge/synopsys/dw-dp.c:1072 dw_dp_send_sdp() warn: bitwise AND is zero '0x1 & 0x2'
>
>drivers/gpu/drm/bridge/synopsys/dw-dp.c
> 1066
> 1067 if (sdp->flags & DW_DP_SDP_VERTICAL_INTERVAL)
> 1068 regmap_update_bits(dp->regmap, DW_DP_SDP_VERTICAL_CTRL,
> 1069 EN_VERTICAL_SDP << nr,
> 1070 EN_VERTICAL_SDP << nr);
> 1071
> 1072 if (sdp->flags & DW_DP_SDP_HORIZONTAL_INTERVAL)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>DW_DP_SDP_HORIZONTAL_INTERVAL is never used.
>
> 1073 regmap_update_bits(dp->regmap, DW_DP_SDP_HORIZONTAL_CTRL,
> 1074 EN_HORIZONTAL_SDP << nr,
> 1075 EN_HORIZONTAL_SDP << nr);
> 1076
>
[0]https://lore.kernel.org/dri-devel/20250902112922.684581-1-andyshrk@163.com/T/#u
>regards,
>dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] drm/bridge: synopsys: Add DW DPTX Controller support library
2025-09-02 11:42 ` Andy Yan
@ 2025-09-02 11:45 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2025-09-02 11:45 UTC (permalink / raw)
To: Andy Yan; +Cc: Andy Yan, dri-devel
On Tue, Sep 02, 2025 at 07:42:25PM +0800, Andy Yan wrote:
> >
> >There is another similar warning but it's probably deliberate.
>
>
> Yes, this is deliberate, we will set DW_DP_SDP_HORIZONTAL_INTERVAL when we add support for audio in the
> future. So I think we can let it as it is now.
>
Sounds good!
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-09-02 11:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-02 6:35 [bug report] drm/bridge: synopsys: Add DW DPTX Controller support library Dan Carpenter
2025-09-02 11:42 ` Andy Yan
2025-09-02 11:45 ` [bug " Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.