Skip to contentSkip to conversation

drm/sun4i: hdmi: Fix sun4i_tmds_determine_rate

PATCH Applied Jonathan Liu 2 participants 5 messages Jan 9, 2018 – Jan 11, 2018

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

v3 cover drm/sun4i: hdmi: Fix sun4i_tmds_determine_rate

This patchset fixes several issues in sun4i_tmds_determine_rate that I discovered while trying to get a projector connected to an Olimex A20-OLinuXino-LIME using HDMI with a native resolution of 1280x800 and pixel clock of 83.5 MHz to display at its native resolution.

Changes for v3:
- Improve commit message for unset best_parent

Changes for v2:
- Split into separate patches for each issue
- Add details to commit message for reproducing issue

Jonathan Liu (3):
drm/sun4i: hdmi: Check for unset best_parent in
sun4i_tmds_determine_rate
drm/sun4i: hdmi: Fix incorrect assignment in sun4i_tmds_determine_rate drm/sun4i: hdmi: Add missing rate halving check in
sun4i_tmds_determine_rate

 drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
Signature · 1 line
2.15.1
#1 lore raw

PATCH v3 1/3 drm/sun4i: hdmi: Check for unset best_parent in sun4i_tmds_determine_rate

It is possible that if there is no exact rate match and "rounded = clk_hw_round_rate(parent, ideal)" gives high enough values (e.g. if rounded is 2 * ideal) that the condition
"abs(rate - rounded / i) < abs(rate - best_parent / best_div)" is never met and best_parent is never set. This results in req->rate and req->best_parent_rate being assigned 0.

To avoid this, we set best_parent to the first calculated rate if it is unset. The sun4i_tmds_calc_divider function already has a similar check.

Fixes 9c5681011a0c ("drm/sun4i: Add HDMI support")
---
 drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c +1 -1
index dc332ea56f6c..4d235e5ea31c 100644
@@ -102,7 +102,7 @@ static int sun4i_tmds_determine_rate(struct clk_hw *hw,
 					goto out;
 				}
 
-				if (abs(rate - rounded / i) <
+				if (!best_parent || abs(rate - rounded / i) <
 				    abs(rate - best_parent / best_div)) {
 					best_parent = rounded;
 					best_div = i;
Signature · 1 line
2.15.1
#2 lore raw

PATCH v3 2/3 drm/sun4i: hdmi: Fix incorrect assignment in sun4i_tmds_determine_rate

In reply to Jonathan Liu

best_div is set to i which corresponds to rate halving when it should be set to j which corresponds to the divider.

Fixes 9c5681011a0c ("drm/sun4i: Add HDMI support")
---
 drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c +1 -1
index 4d235e5ea31c..88eeeaf34638 100644
@@ -105,7 +105,7 @@ static int sun4i_tmds_determine_rate(struct clk_hw *hw,
 				if (!best_parent || abs(rate - rounded / i) <
 				    abs(rate - best_parent / best_div)) {
 					best_parent = rounded;
-					best_div = i;
+					best_div = j;
 				}
 			}
 		}
Signature · 1 line
2.15.1
#3 lore raw

PATCH v3 3/3 drm/sun4i: hdmi: Add missing rate halving check in sun4i_tmds_determine_rate

In reply to Jonathan Liu

It was only checking the divider when determing the closest match if it could not match the requested rate exactly.

For a projector connected to an Olimex A20-OLinuXino-LIME using HDMI
with a native resolution of 1280x800 and pixel clock of 83.5 MHz, this
resulted in 1280x800 mode not being available and the following in dmesg
when the kernel is booted with drm.debug=0x3e:
[drm:drm_mode_debug_printmodeline] Modeline 37:"1280x800" 60 83500 1280 1352 1480 1680 800 810 816 831 0x48 0x5 [drm:drm_mode_prune_invalid] Not using 1280x800 mode: NOCLOCK

Fixes 9c5681011a0c ("drm/sun4i: Add HDMI support")
---
 drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c +5 -2
index 88eeeaf34638..3ecffa52c814 100644
@@ -102,9 +102,12 @@ static int sun4i_tmds_determine_rate(struct clk_hw *hw,
 					goto out;
 				}
 
-				if (!best_parent || abs(rate - rounded / i) <
-				    abs(rate - best_parent / best_div)) {
+				if (!best_parent ||
+				    abs(rate - rounded / i / j) <
+				    abs(rate - best_parent / best_half /
+					best_div)) {
 					best_parent = rounded;
+					best_half = i;
 					best_div = j;
 				}
 			}
Signature · 1 line
2.15.1
#4 lore raw
In reply to Jonathan Liu
Quoted 4 lines · Jonathan Liu

This patchset fixes several issues in sun4i_tmds_determine_rate that I discovered while trying to get a projector connected to an Olimex A20-OLinuXino-LIME using HDMI with a native resolution of 1280x800 and pixel clock of 83.5 MHz to display at its native resolution.

Queued all three patches as fixes, thanks!
maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180111/6bfb307b/attachment-0001.sig>