[PATCH 2/2] Allow/Fix use of multiple ZaphodHead outputs per x-screen.

[Michel Dänzer]

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 <mario.kleiner.de at gmail.com>

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