* Re: Radeon monitor + hdmi TV regression between drm-core-next and drm-fixes
2012-11-04 21:00 ` Andy Furniss
@ 2012-11-05 15:56 ` Alex Deucher
2012-11-05 17:54 ` Andy Furniss
0 siblings, 1 reply; 5+ messages in thread
From: Alex Deucher @ 2012-11-05 15:56 UTC (permalink / raw)
To: Andy Furniss; +Cc: dri-devel
[-- Attachment #1: Type: text/plain, Size: 2405 bytes --]
On Sun, Nov 4, 2012 at 4:00 PM, Andy Furniss <andyqos@ukfsn.org> wrote:
> Alex Deucher wrote:
>>
>> On Sun, Nov 4, 2012 at 10:27 AM, Andy Furniss <andyqos@ukfsn.org> wrote:
>>>
>>> For the last 2 years when running a DVI 60Hz monitor with a radeon HD4890
>>> and a (native 50Hz) HDMI TV I've been able to boot/startx with the TV off
>>> and then turn TV on and -
>>>
>>> xrandr --output DVI-0 --auto
>>>
>>> to bring up the the TV and get a clone of monitor.
>>>
>>> This still works with drm-core-next but not with drm-fixes (todays or
>>> from a
>>> few days ago).
>>>
>>> With df I now loose the monitor with signal out of range when doing
>>> above,
>>> the TV output is OK. To get the monitor back I need to turn off TV, then
>>> off/auto the monitor.
>>>
>>> xrandr --output DVI-0 --off
>>> xrandr --output DVI-1 --off
>>> xrandr --output DVI-1 --auto
>>>
>>> The output from xrandr while the monitor is showing signal out of range
>>> looks normal.
>>>
>>> If I boot with the TV on it works OK.
>>
>>
>> Can you bisect?
>
>
> 29dbe3bcd2e28e71823febdca989d63d5c27d152 is the first bad commit
> commit 29dbe3bcd2e28e71823febdca989d63d5c27d152
> Author: Alex Deucher <alexander.deucher@amd.com>
> Date: Fri Oct 5 10:22:02 2012 -0400
>
> drm/radeon: allocate PPLLs from low to high
>
> The order shouldn't matter, but there have been problems
> reported on certain older asics. This behaves more
> like the original code before the PPLL allocation
> rework.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: Markus Trippelsdorf <markus@trippelsdorf.de>
>
>
That's bizarre. That patch reverts the behavior back to the 3.6 and
earlier kernel behavior. I guess it's some issue with the ordering of
the modesetting programming sequence. I've attached a couple of
things to try.
The first patch is a simple fix. It just reverts back to the previous
pll allocation order for discrete cards like yours:
0001-drm-radeon-dce3-switch-back-to-old-pll-allocation-or.patch
The second set of patches implements a more complex fix which may help
regardless of the order in which plls are allocated:
0001-drm-radeon-split-out-the-pll-disable-into-a-helper-f.patch
0002-drm-radeon-add-a-helper-to-check-if-a-pll-is-shared.patch
0003-drm-radeon-disable-the-pll-before-a-modeset.patch
Can you see if the second set helps? If not, please try the first patch.
Alex
[-- Attachment #2: 0001-drm-radeon-dce3-switch-back-to-old-pll-allocation-or.patch --]
[-- Type: text/x-patch, Size: 3154 bytes --]
From b6cd7967513fe86c0822db4b41745da2140914b7 Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexander.deucher@amd.com>
Date: Mon, 5 Nov 2012 10:16:12 -0500
Subject: [PATCH] drm/radeon/dce3: switch back to old pll allocation order for
discrete
The order shouldn't matter, but this seems to cause regressions for
certain specific cases. This should fix it for now. We probably
need to investigate a proper fix in the next development cycle.
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: Andy Furniss <andyqos@ukfsn.org>
---
drivers/gpu/drm/radeon/atombios_crtc.c | 54 ++++++++++++++++++-------------
1 files changed, 31 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index 2e566e1..3bce029 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -1696,35 +1696,43 @@ static int radeon_atom_pick_pll(struct drm_crtc *crtc)
return ATOM_PPLL2;
DRM_ERROR("unable to allocate a PPLL\n");
return ATOM_PPLL_INVALID;
- } else {
- if (ASIC_IS_AVIVO(rdev)) {
- /* in DP mode, the DP ref clock can come from either PPLL
- * depending on the asic:
- * DCE3: PPLL1 or PPLL2
- */
- if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(radeon_crtc->encoder))) {
- /* use the same PPLL for all DP monitors */
- pll = radeon_get_shared_dp_ppll(crtc);
- if (pll != ATOM_PPLL_INVALID)
- return pll;
- } else {
- /* use the same PPLL for all monitors with the same clock */
- pll = radeon_get_shared_nondp_ppll(crtc);
- if (pll != ATOM_PPLL_INVALID)
- return pll;
- }
- /* all other cases */
- pll_in_use = radeon_get_pll_use_mask(crtc);
+ } else if (ASIC_IS_AVIVO(rdev)) {
+ /* in DP mode, the DP ref clock can come from either PPLL
+ * depending on the asic:
+ * DCE3: PPLL1 or PPLL2
+ */
+ if (ENCODER_MODE_IS_DP(atombios_get_encoder_mode(radeon_crtc->encoder))) {
+ /* use the same PPLL for all DP monitors */
+ pll = radeon_get_shared_dp_ppll(crtc);
+ if (pll != ATOM_PPLL_INVALID)
+ return pll;
+ } else {
+ /* use the same PPLL for all monitors with the same clock */
+ pll = radeon_get_shared_nondp_ppll(crtc);
+ if (pll != ATOM_PPLL_INVALID)
+ return pll;
+ }
+ /* all other cases */
+ pll_in_use = radeon_get_pll_use_mask(crtc);
+ /* the order shouldn't matter here, but we probably
+ * need this until we have atomic modeset
+ */
+ if (rdev->flags & RADEON_IS_IGP) {
if (!(pll_in_use & (1 << ATOM_PPLL1)))
return ATOM_PPLL1;
if (!(pll_in_use & (1 << ATOM_PPLL2)))
return ATOM_PPLL2;
- DRM_ERROR("unable to allocate a PPLL\n");
- return ATOM_PPLL_INVALID;
} else {
- /* on pre-R5xx asics, the crtc to pll mapping is hardcoded */
- return radeon_crtc->crtc_id;
+ if (!(pll_in_use & (1 << ATOM_PPLL2)))
+ return ATOM_PPLL2;
+ if (!(pll_in_use & (1 << ATOM_PPLL1)))
+ return ATOM_PPLL1;
}
+ DRM_ERROR("unable to allocate a PPLL\n");
+ return ATOM_PPLL_INVALID;
+ } else {
+ /* on pre-R5xx asics, the crtc to pll mapping is hardcoded */
+ return radeon_crtc->crtc_id;
}
}
--
1.7.7.5
[-- Attachment #3: 0001-drm-radeon-split-out-the-pll-disable-into-a-helper-f.patch --]
[-- Type: text/x-patch, Size: 3601 bytes --]
From 9ba29cd4eef71624ad8a02291dbdc3a3199be59d Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexander.deucher@amd.com>
Date: Mon, 5 Nov 2012 10:28:55 -0500
Subject: [PATCH 1/3] drm/radeon: split out the pll disable into a helper
function
This will be used later to share code for disabling the pll.
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/radeon/atombios_crtc.c | 74 +++++++++++++++++++-------------
1 files changed, 44 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index 3bce029..0a35709 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -1828,32 +1828,15 @@ static bool atombios_crtc_mode_fixup(struct drm_crtc *crtc,
return true;
}
-static void atombios_crtc_prepare(struct drm_crtc *crtc)
-{
- struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
- struct drm_device *dev = crtc->dev;
- struct radeon_device *rdev = dev->dev_private;
-
- radeon_crtc->in_mode_set = true;
-
- /* disable crtc pair power gating before programming */
- if (ASIC_IS_DCE6(rdev))
- atombios_powergate_crtc(crtc, ATOM_DISABLE);
-
- atombios_lock_crtc(crtc, ATOM_ENABLE);
- atombios_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
-}
-
-static void atombios_crtc_commit(struct drm_crtc *crtc)
-{
- struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
-
- atombios_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
- atombios_lock_crtc(crtc, ATOM_DISABLE);
- radeon_crtc->in_mode_set = false;
-}
-
-static void atombios_crtc_disable(struct drm_crtc *crtc)
+/**
+ * atombios_disable_pll - turn off a display PLL
+ *
+ * @crtc: drm crtc
+ *
+ * Disable the PLL currently assigned to the crtc (all asics).
+ * Skip disabling the PLL if it's shared with another crtc.
+ */
+static void atombios_disable_pll(struct drm_crtc *crtc)
{
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct drm_device *dev = crtc->dev;
@@ -1861,8 +1844,6 @@ static void atombios_crtc_disable(struct drm_crtc *crtc)
struct radeon_atom_ss ss;
int i;
- atombios_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
-
for (i = 0; i < rdev->num_crtc; i++) {
if (rdev->mode_info.crtcs[i] &&
rdev->mode_info.crtcs[i]->enabled &&
@@ -1871,7 +1852,7 @@ static void atombios_crtc_disable(struct drm_crtc *crtc)
/* one other crtc is using this pll don't turn
* off the pll
*/
- goto done;
+ return;
}
}
@@ -1891,7 +1872,40 @@ static void atombios_crtc_disable(struct drm_crtc *crtc)
default:
break;
}
-done:
+}
+
+static void atombios_crtc_prepare(struct drm_crtc *crtc)
+{
+ struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
+ struct drm_device *dev = crtc->dev;
+ struct radeon_device *rdev = dev->dev_private;
+
+ radeon_crtc->in_mode_set = true;
+
+ /* disable crtc pair power gating before programming */
+ if (ASIC_IS_DCE6(rdev))
+ atombios_powergate_crtc(crtc, ATOM_DISABLE);
+
+ atombios_lock_crtc(crtc, ATOM_ENABLE);
+ atombios_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
+}
+
+static void atombios_crtc_commit(struct drm_crtc *crtc)
+{
+ struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
+
+ atombios_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
+ atombios_lock_crtc(crtc, ATOM_DISABLE);
+ radeon_crtc->in_mode_set = false;
+}
+
+static void atombios_crtc_disable(struct drm_crtc *crtc)
+{
+ struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
+
+ atombios_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
+ atombios_disable_pll(crtc);
+
radeon_crtc->pll_id = ATOM_PPLL_INVALID;
radeon_crtc->adjusted_clock = 0;
radeon_crtc->encoder = NULL;
--
1.7.7.5
[-- Attachment #4: 0002-drm-radeon-add-a-helper-to-check-if-a-pll-is-shared.patch --]
[-- Type: text/x-patch, Size: 3167 bytes --]
From cb637523b3fff880e37cc474266f62cb1feb4ecb Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexander.deucher@amd.com>
Date: Mon, 5 Nov 2012 10:41:48 -0500
Subject: [PATCH 2/3] drm/radeon: add a helper to check if a pll is shared
reduces duplicated code.
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/radeon/atombios_crtc.c | 63 +++++++++++++++++++-------------
1 files changed, 38 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index 0a35709..b072c1c 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -408,6 +408,33 @@ static void atombios_disable_ss(struct radeon_device *rdev, int pll_id)
}
}
+/**
+ * atombios_pll_shared - check if a pll is shared
+ *
+ * @rdev: radeon_device pointer
+ * @crtc_id: id of the requested crtc
+ * @pll_id: id of the requested pll
+ *
+ * Checks if a pll used a particular crtc is shared
+ * with another crtc. Returns true if the pll is shared,
+ * false if not.
+ */
+static bool atombios_pll_shared(struct radeon_device *rdev,
+ int crtc_id, int pll_id)
+{
+ int i;
+
+ for (i = 0; i < rdev->num_crtc; i++) {
+ if (rdev->mode_info.crtcs[i] &&
+ rdev->mode_info.crtcs[i]->enabled &&
+ i != crtc_id &&
+ pll_id == rdev->mode_info.crtcs[i]->pll_id) {
+ /* pll is shared with another crtc */
+ return false;
+ }
+ }
+ return false;
+}
union atom_enable_ss {
ENABLE_LVDS_SS_PARAMETERS lvds_ss;
@@ -423,23 +450,16 @@ static void atombios_crtc_program_ss(struct radeon_device *rdev,
int crtc_id,
struct radeon_atom_ss *ss)
{
- unsigned i;
int index = GetIndexIntoMasterTable(COMMAND, EnableSpreadSpectrumOnPPLL);
union atom_enable_ss args;
if (!enable) {
- for (i = 0; i < rdev->num_crtc; i++) {
- if (rdev->mode_info.crtcs[i] &&
- rdev->mode_info.crtcs[i]->enabled &&
- i != crtc_id &&
- pll_id == rdev->mode_info.crtcs[i]->pll_id) {
- /* one other crtc is using this pll don't turn
- * off spread spectrum as it might turn off
- * display on active crtc
- */
- return;
- }
- }
+ /* one other crtc is using this pll don't turn
+ * off spread spectrum as it might turn off
+ * display on active crtc
+ */
+ if (atombios_pll_shared(rdev, crtc_id, pll_id))
+ return;
}
memset(&args, 0, sizeof(args));
@@ -1842,19 +1862,12 @@ static void atombios_disable_pll(struct drm_crtc *crtc)
struct drm_device *dev = crtc->dev;
struct radeon_device *rdev = dev->dev_private;
struct radeon_atom_ss ss;
- int i;
- for (i = 0; i < rdev->num_crtc; i++) {
- if (rdev->mode_info.crtcs[i] &&
- rdev->mode_info.crtcs[i]->enabled &&
- i != radeon_crtc->crtc_id &&
- radeon_crtc->pll_id == rdev->mode_info.crtcs[i]->pll_id) {
- /* one other crtc is using this pll don't turn
- * off the pll
- */
- return;
- }
- }
+ /* one other crtc is using this pll don't turn
+ * off the pll
+ */
+ if (atombios_pll_shared(rdev, radeon_crtc->crtc_id, radeon_crtc->pll_id))
+ return;
switch (radeon_crtc->pll_id) {
case ATOM_PPLL1:
--
1.7.7.5
[-- Attachment #5: 0003-drm-radeon-disable-the-pll-before-a-modeset.patch --]
[-- Type: text/x-patch, Size: 911 bytes --]
From f7a54adf9b66384afa16fcd00b4602f82e4b6ae9 Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexander.deucher@amd.com>
Date: Mon, 5 Nov 2012 10:44:53 -0500
Subject: [PATCH 3/3] drm/radeon: disable the pll before a modeset
May fix display issues in certain cases.
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/radeon/atombios_crtc.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index b072c1c..62a72fd 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -1901,6 +1901,7 @@ static void atombios_crtc_prepare(struct drm_crtc *crtc)
atombios_lock_crtc(crtc, ATOM_ENABLE);
atombios_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
+ atombios_disable_pll(crtc);
}
static void atombios_crtc_commit(struct drm_crtc *crtc)
--
1.7.7.5
[-- Attachment #6: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread