Re: [PATCH 2/2] Allow/Fix use of multiple ZaphodHead outputs per x-screen.
On 05.06.2015 21:33, Mario Kleiner wrote:
> Defining multiple ZaphodHead outputs per x-screen in a
> multiple x-screen's per gpu configuration caused all
> outputs except one per x-screen to go dark, because
> there was a fixed mapping x-screen number -> crtc number,
> limiting the number of crtc's per x-screen to one.
>
> On a ZaphodHead's setup, be more clever and assign
> as many crtc's to a given x-screen as there are
> ZaphodHeads defined for that screen, assuming
> there are enough unused crtc's available.
>
> Tested on a triple display setup with different combos
> of one, two or three ZaphodHeads per one, two or three
> x-screens.
>
> This is a port of almost identical code from
> nouveau-ddx.
>
> Signed-off-by: Mario Kleiner
Some questions / suggestions below. Other than that, it looks good to me.
> + /* Mark num'th crtc as in use on this device. */
> + pRADEONEnt->assigned_crtcs |= (1 << num);
> + xf86DrvMsg(pScrn->scrnIndex, X_INFO,
> +"Allocated crtc nr. %d to this screen.\n", num);
These log messages seem rather verbose. Maybe they should have a higher
log level so they aren't printed by default.
> + /* All ZaphodHeads outputs provided with matching crtcs? */
> + if (xf86IsEntityShared(pScrn->entityList[0]) && (crtcs_needed > 0))
> + xf86DrvMsg(pScrn->scrnIndex, X_WARNING,
> +"%d ZaphodHeads crtcs unavailable. Trouble!\n",
> crtcs_needed);
Can this say something more useful than "Trouble!", e.g. what bad
thing(s) will likely happen?
> diff --git a/src/radeon_probe.c b/src/radeon_probe.c
> index ad1e96e..45a89f3 100644
> --- a/src/radeon_probe.c
> +++ b/src/radeon_probe.c
> @@ -174,6 +174,13 @@ radeon_get_scrninfo(int entity_num, void *pci_dev)
> pRADEONEnt = pPriv->ptr;
> pRADEONEnt->HasSecondary = TRUE;
> }
> +
> +/* Reset settings which must not persist across server regeneration
> */
> +if (pRADEONEnt->reinitGeneration != serverGeneration) {
> +pRADEONEnt->reinitGeneration = serverGeneration;
> +/* Clear mask of assigned crtc's in this generation to "none" */
> +pRADEONEnt->assigned_crtcs = 0;
> +}
Is pRADEONEnt->reinitGeneration really necessary? Couldn't we just set
pRADEONEnt->assigned_crtcs = 0 in CloseScreen?
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-driver-ati mailing list
xorg-driver-ati@lists.x.org
http://lists.x.org/mailman/listinfo/xorg-driver-ati
Re: [PATCH 2/2] Allow/Fix use of multiple ZaphodHead outputs per x-screen.
On Fri, Jun 5, 2015 at 8:33 AM, Mario Kleiner
wrote:
> Defining multiple ZaphodHead outputs per x-screen in a
> multiple x-screen's per gpu configuration caused all
> outputs except one per x-screen to go dark, because
> there was a fixed mapping x-screen number -> crtc number,
> limiting the number of crtc's per x-screen to one.
>
> On a ZaphodHead's setup, be more clever and assign
> as many crtc's to a given x-screen as there are
> ZaphodHeads defined for that screen, assuming
> there are enough unused crtc's available.
>
> Tested on a triple display setup with different combos
> of one, two or three ZaphodHeads per one, two or three
> x-screens.
>
> This is a port of almost identical code from
> nouveau-ddx.
>
> Signed-off-by: Mario Kleiner
Reviewed-by: Alex Deucher
> ---
> src/drmmode_display.c | 44 +++-
> src/radeon_probe.c| 15 +++
> src/radeon_probe.h| 2 ++
> 3 files changed, 48 insertions(+), 13 deletions(-)
>
> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> index c12bf51..a9f5481 100644
> --- a/src/drmmode_display.c
> +++ b/src/drmmode_display.c
> @@ -1012,15 +1012,16 @@ void drmmode_crtc_hw_id(xf86CrtcPtr crtc)
> drmmode_crtc->hw_id = tmp;
> }
>
> -static void
> +static unsigned int
> drmmode_crtc_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, drmModeResPtr
> mode_res, int num)
> {
> xf86CrtcPtr crtc;
> drmmode_crtc_private_ptr drmmode_crtc;
> + RADEONEntPtr pRADEONEnt = RADEONEntPriv(pScrn);
>
> crtc = xf86CrtcCreate(pScrn, &drmmode_crtc_funcs);
> if (crtc == NULL)
> - return;
> + return 0;
>
> drmmode_crtc = xnfcalloc(sizeof(drmmode_crtc_private_rec), 1);
> drmmode_crtc->mode_crtc = drmModeGetCrtc(drmmode->fd,
> mode_res->crtcs[num]);
> @@ -1028,7 +1029,12 @@ drmmode_crtc_init(ScrnInfoPtr pScrn, drmmode_ptr
> drmmode, drmModeResPtr mode_res
> crtc->driver_private = drmmode_crtc;
> drmmode_crtc_hw_id(crtc);
>
> - return;
> + /* Mark num'th crtc as in use on this device. */
> + pRADEONEnt->assigned_crtcs |= (1 << num);
> + xf86DrvMsg(pScrn->scrnIndex, X_INFO,
> + "Allocated crtc nr. %d to this screen.\n", num);
> +
> + return 1;
> }
>
> static xf86OutputStatus
> @@ -1461,7 +1467,7 @@ drmmode_create_name(ScrnInfoPtr pScrn,
> drmModeConnectorPtr koutput, char *name,
> }
> }
>
> -static void
> +static unsigned int
> drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, drmModeResPtr
> mode_res, int num, int *num_dvi, int *num_hdmi, int dynamic)
> {
> xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(pScrn);
> @@ -1478,7 +1484,7 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr
> drmmode, drmModeResPtr mode_r
>
> koutput = drmModeGetConnector(drmmode->fd, mode_res->connectors[num]);
> if (!koutput)
> - return;
> + return 0;
>
> for (i = 0; i < koutput->count_props; i++) {
> props = drmModeGetProperty(drmmode->fd, koutput->props[i]);
> @@ -1524,7 +1530,7 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr
> drmmode, drmModeResPtr mode_r
> for (i = 0; i < koutput->count_encoders; i++)
> drmModeFreeEncoder(kencoders[i]);
> free(kencoders);
> - return;
> + return 0;
> }
> }
>
> @@ -1587,7 +1593,7 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr
> drmmode, drmModeResPtr mode_r
> drmmode_output_create_resources(output);
> }
>
> - return;
> + return 1;
> out_free_encoders:
> if (kencoders){
> for (i = 0; i < koutput->count_encoders; i++)
> @@ -1595,7 +1601,7 @@ out_free_encoders:
> free(kencoders);
> }
> drmModeFreeConnector(koutput);
> -
> + return 0;
> }
>
> uint32_t find_clones(ScrnInfoPtr scrn, xf86OutputPtr output)
> @@ -2030,8 +2036,10 @@ drm_wakeup_handler(pointer data, int err, pointer p)
>
> Bool drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int cpp)
> {
> + RADEONEntPtr pRADEONEnt = RADEONEntPriv(pScrn);
> int i, num_dvi = 0, num_hdmi = 0;
> drmModeResPtr mode_res;
> + unsigned int crtcs_needed = 0;
>
> xf86CrtcConfigInit(pScrn, &drmmode_xf86crtc_config_funcs);
>
> @@ -2041,14 +2049,24 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr
> drmmode, int cpp)
> if (!mode_res)
> return FALSE;
>
> + xf86DrvMsg(pScrn->scrnIndex, X_INFO, "Initializing outputs ...\n");
> + for (i = 0; i < mode_res->count_connectors; i++)
> + crtcs_needed += drmmode_output_init(pScrn, drmmode, mode_res,
> + i, &num_dvi, &num_hdmi,
> 0);
> + xf86DrvMsg(pScrn->scrnIndex, X
[PATCH 2/2] Allow/Fix use of multiple ZaphodHead outputs per x-screen.
Defining multiple ZaphodHead outputs per x-screen in a
multiple x-screen's per gpu configuration caused all
outputs except one per x-screen to go dark, because
there was a fixed mapping x-screen number -> crtc number,
limiting the number of crtc's per x-screen to one.
On a ZaphodHead's setup, be more clever and assign
as many crtc's to a given x-screen as there are
ZaphodHeads defined for that screen, assuming
there are enough unused crtc's available.
Tested on a triple display setup with different combos
of one, two or three ZaphodHeads per one, two or three
x-screens.
This is a port of almost identical code from
nouveau-ddx.
Signed-off-by: Mario Kleiner
---
src/drmmode_display.c | 44 +++-
src/radeon_probe.c| 15 +++
src/radeon_probe.h| 2 ++
3 files changed, 48 insertions(+), 13 deletions(-)
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index c12bf51..a9f5481 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -1012,15 +1012,16 @@ void drmmode_crtc_hw_id(xf86CrtcPtr crtc)
drmmode_crtc->hw_id = tmp;
}
-static void
+static unsigned int
drmmode_crtc_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, drmModeResPtr
mode_res, int num)
{
xf86CrtcPtr crtc;
drmmode_crtc_private_ptr drmmode_crtc;
+ RADEONEntPtr pRADEONEnt = RADEONEntPriv(pScrn);
crtc = xf86CrtcCreate(pScrn, &drmmode_crtc_funcs);
if (crtc == NULL)
- return;
+ return 0;
drmmode_crtc = xnfcalloc(sizeof(drmmode_crtc_private_rec), 1);
drmmode_crtc->mode_crtc = drmModeGetCrtc(drmmode->fd,
mode_res->crtcs[num]);
@@ -1028,7 +1029,12 @@ drmmode_crtc_init(ScrnInfoPtr pScrn, drmmode_ptr
drmmode, drmModeResPtr mode_res
crtc->driver_private = drmmode_crtc;
drmmode_crtc_hw_id(crtc);
- return;
+ /* Mark num'th crtc as in use on this device. */
+ pRADEONEnt->assigned_crtcs |= (1 << num);
+ xf86DrvMsg(pScrn->scrnIndex, X_INFO,
+ "Allocated crtc nr. %d to this screen.\n", num);
+
+ return 1;
}
static xf86OutputStatus
@@ -1461,7 +1467,7 @@ drmmode_create_name(ScrnInfoPtr pScrn,
drmModeConnectorPtr koutput, char *name,
}
}
-static void
+static unsigned int
drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, drmModeResPtr
mode_res, int num, int *num_dvi, int *num_hdmi, int dynamic)
{
xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(pScrn);
@@ -1478,7 +1484,7 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr
drmmode, drmModeResPtr mode_r
koutput = drmModeGetConnector(drmmode->fd, mode_res->connectors[num]);
if (!koutput)
- return;
+ return 0;
for (i = 0; i < koutput->count_props; i++) {
props = drmModeGetProperty(drmmode->fd, koutput->props[i]);
@@ -1524,7 +1530,7 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr
drmmode, drmModeResPtr mode_r
for (i = 0; i < koutput->count_encoders; i++)
drmModeFreeEncoder(kencoders[i]);
free(kencoders);
- return;
+ return 0;
}
}
@@ -1587,7 +1593,7 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr
drmmode, drmModeResPtr mode_r
drmmode_output_create_resources(output);
}
- return;
+ return 1;
out_free_encoders:
if (kencoders){
for (i = 0; i < koutput->count_encoders; i++)
@@ -1595,7 +1601,7 @@ out_free_encoders:
free(kencoders);
}
drmModeFreeConnector(koutput);
-
+ return 0;
}
uint32_t find_clones(ScrnInfoPtr scrn, xf86OutputPtr output)
@@ -2030,8 +2036,10 @@ drm_wakeup_handler(pointer data, int err, pointer p)
Bool drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int cpp)
{
+ RADEONEntPtr pRADEONEnt = RADEONEntPriv(pScrn);
int i, num_dvi = 0, num_hdmi = 0;
drmModeResPtr mode_res;
+ unsigned int crtcs_needed = 0;
xf86CrtcConfigInit(pScrn, &drmmode_xf86crtc_config_funcs);
@@ -2041,14 +2049,24 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr
drmmode, int cpp)
if (!mode_res)
return FALSE;
+ xf86DrvMsg(pScrn->scrnIndex, X_INFO, "Initializing outputs ...\n");
+ for (i = 0; i < mode_res->count_connectors; i++)
+ crtcs_needed += drmmode_output_init(pScrn, drmmode, mode_res,
+ i, &num_dvi, &num_hdmi, 0);
+ xf86DrvMsg(pScrn->scrnIndex, X_INFO,
+ "%d crtcs needed for screen.\n", crtcs_needed);
+
drmmode->count_crtcs = mode_res->count_crtcs;
xf86CrtcSetSizeRange(pScrn, 320, 200, mode_res->max_width,
mode_res->max_height);
for (i = 0; i < mode_res->count_crtcs; i++)
- if (!xf86IsEntityShared(pSc
