All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Take shift in crtc positions for ZaphodHeads configs into account.
@ 2015-06-28  0:33 Mario Kleiner
       [not found] ` <1435451629-17959-1-git-send-email-mario.kleiner.de-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 4+ messages in thread
From: Mario Kleiner @ 2015-06-28  0:33 UTC (permalink / raw)
  To: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: mario.kleiner.de-Re5JQEeQqe8AvxtiuMwx3w,
	bskeggs-H+wXaHxf7aLQT0dZR+AlfA

In multi-x-screen ZaphodHeads configurations, there isn't a
one-to-one mapping of kernel provided drmmode crtc index
to the index of the corresponding xf86Crtc inside the
xf86CrtcConfig crtc array anymore, ie. for kernel provided
drmmode->mode_res->crtcs[i], the i'th crtc won't correspond
to the xf86Crtc in the i'th slot of the x-screens xf86CrtcConfig
anymore, once ZaphodHeads has only selected a subset of all crtcs
of a graphics card for a given x-screen, instead of all crtcs.

This breaks the mapping of bit positions in the bit masks returned
in kencoder->possible_crtcs and kencoder->possible_clones. A 1 bit
in position i of those masks allows use of the kernels i'th crtc for
the given kencoder. The X-Servers dix code checks those bit masks
for valid xf86Output -> xf86Crtc assignments, assuming that the i'th
slot xf86CrtcConfigPtr config->crtc[i] corresponds to bit i in the
xf86Output->possibe_crtcs bitmask, and bails if the bitmask doesn't
allow the specified assignment of crtc to output. If ZaphodHeads
breaks the assumption of bit i <-> crtc slot i this ends in failure.

Take this shift of crtc index positions wrt. encoder bitmask bit
positions into account by bit-shifting positions accordingly when
assigning encoder->possible_crtcs to output->possible_crtcs, so
the proper indices match up again for validation by the dix.

This problem wasn't apparent last year when testing the ZaphodHeads
support on some Kepler cards, as apparently the encoder->possible_crtcs
bitmasks returned for those cards by the kernel just had all 4
lsb bits set for all tested encoders/output, so each of the cards 4
crtcs could go with each output and things worked by chance.

The current code breaks, e.g., on 2010 MacBookPro with nv50, where
one crtc is hardwired to the internal lvds panel, and one crtc
is hardwired to the external DP connector, resulting in a failure
where dual-display on single-x-screen works fine, but assigning
each output to a separate x-screen via ZaphodHeads fails due to
the mismatched encoder->possible_crtcs bitmasks.

This patch fixes the problem.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
---
 src/drmmode_display.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index c30cb3a..3679482 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -1214,7 +1214,7 @@ drmmode_zaphod_match(ScrnInfoPtr pScrn, const char *s, char *output_name)
 }
 
 static unsigned int
-drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num)
+drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num, int crtcshift)
 {
 	NVPtr pNv = NVPTR(pScrn);
 	xf86OutputPtr output;
@@ -1296,8 +1296,8 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num)
 	output->subpixel_order = subpixel_conv_table[koutput->subpixel];
 	output->driver_private = drmmode_output;
 
-	output->possible_crtcs = kencoder->possible_crtcs;
-	output->possible_clones = kencoder->possible_clones;
+	output->possible_crtcs = kencoder->possible_crtcs >> crtcshift;
+	output->possible_clones = kencoder->possible_clones >> crtcshift;
 
 	output->interlaceAllowed = true;
 	output->doubleScanAllowed = true;
@@ -1421,6 +1421,7 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, int fd, int cpp)
 	NVEntPtr pNVEnt = NVEntPriv(pScrn);
 	int i;
 	unsigned int crtcs_needed = 0;
+	int crtcshift;
 
 	drmmode = xnfalloc(sizeof *drmmode);
 	drmmode->fd = fd;
@@ -1444,8 +1445,9 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, int fd, int cpp)
 	}
 
 	xf86DrvMsg(pScrn->scrnIndex, X_INFO, "Initializing outputs ...\n");
+	crtcshift = ffs(pNVEnt->assigned_crtcs ^ 0xffffffff) - 1;
 	for (i = 0; i < drmmode->mode_res->count_connectors; i++)
-		crtcs_needed += drmmode_output_init(pScrn, drmmode, i);
+		crtcs_needed += drmmode_output_init(pScrn, drmmode, i, crtcshift);
 
 	xf86DrvMsg(pScrn->scrnIndex, X_INFO,
 		   "%d crtcs needed for screen.\n", crtcs_needed);
-- 
1.9.1

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Take shift in crtc positions for ZaphodHeads configs into account.
       [not found] ` <1435451629-17959-1-git-send-email-mario.kleiner.de-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2015-08-06 17:34   ` Ilia Mirkin
       [not found]     ` <CAKb7UvgWqo7HH+-fuO+FTSqrusotFcMoCG6Juc2J5ObM9QRpYQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 4+ messages in thread
From: Ilia Mirkin @ 2015-08-06 17:34 UTC (permalink / raw)
  To: Mario Kleiner
  Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	Ben Skeggs

I don't understand this patch (what are all these masks? how are they
used?), and don't want to invest the time required to do so.

However Mario is probably the sole serious user of ZaphodHeads, and if
it fixes issues for him, probably fixes issues for others who try and
give up with ZaphodHeads. Any objections if I just push this out?

On Sat, Jun 27, 2015 at 8:33 PM, Mario Kleiner
<mario.kleiner.de@gmail.com> wrote:
> In multi-x-screen ZaphodHeads configurations, there isn't a
> one-to-one mapping of kernel provided drmmode crtc index
> to the index of the corresponding xf86Crtc inside the
> xf86CrtcConfig crtc array anymore, ie. for kernel provided
> drmmode->mode_res->crtcs[i], the i'th crtc won't correspond
> to the xf86Crtc in the i'th slot of the x-screens xf86CrtcConfig
> anymore, once ZaphodHeads has only selected a subset of all crtcs
> of a graphics card for a given x-screen, instead of all crtcs.
>
> This breaks the mapping of bit positions in the bit masks returned
> in kencoder->possible_crtcs and kencoder->possible_clones. A 1 bit
> in position i of those masks allows use of the kernels i'th crtc for
> the given kencoder. The X-Servers dix code checks those bit masks
> for valid xf86Output -> xf86Crtc assignments, assuming that the i'th
> slot xf86CrtcConfigPtr config->crtc[i] corresponds to bit i in the
> xf86Output->possibe_crtcs bitmask, and bails if the bitmask doesn't
> allow the specified assignment of crtc to output. If ZaphodHeads
> breaks the assumption of bit i <-> crtc slot i this ends in failure.
>
> Take this shift of crtc index positions wrt. encoder bitmask bit
> positions into account by bit-shifting positions accordingly when
> assigning encoder->possible_crtcs to output->possible_crtcs, so
> the proper indices match up again for validation by the dix.
>
> This problem wasn't apparent last year when testing the ZaphodHeads
> support on some Kepler cards, as apparently the encoder->possible_crtcs
> bitmasks returned for those cards by the kernel just had all 4
> lsb bits set for all tested encoders/output, so each of the cards 4
> crtcs could go with each output and things worked by chance.
>
> The current code breaks, e.g., on 2010 MacBookPro with nv50, where
> one crtc is hardwired to the internal lvds panel, and one crtc
> is hardwired to the external DP connector, resulting in a failure
> where dual-display on single-x-screen works fine, but assigning
> each output to a separate x-screen via ZaphodHeads fails due to
> the mismatched encoder->possible_crtcs bitmasks.
>
> This patch fixes the problem.
>
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> ---
>  src/drmmode_display.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> index c30cb3a..3679482 100644
> --- a/src/drmmode_display.c
> +++ b/src/drmmode_display.c
> @@ -1214,7 +1214,7 @@ drmmode_zaphod_match(ScrnInfoPtr pScrn, const char *s, char *output_name)
>  }
>
>  static unsigned int
> -drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num)
> +drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num, int crtcshift)
>  {
>         NVPtr pNv = NVPTR(pScrn);
>         xf86OutputPtr output;
> @@ -1296,8 +1296,8 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num)
>         output->subpixel_order = subpixel_conv_table[koutput->subpixel];
>         output->driver_private = drmmode_output;
>
> -       output->possible_crtcs = kencoder->possible_crtcs;
> -       output->possible_clones = kencoder->possible_clones;
> +       output->possible_crtcs = kencoder->possible_crtcs >> crtcshift;
> +       output->possible_clones = kencoder->possible_clones >> crtcshift;
>
>         output->interlaceAllowed = true;
>         output->doubleScanAllowed = true;
> @@ -1421,6 +1421,7 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, int fd, int cpp)
>         NVEntPtr pNVEnt = NVEntPriv(pScrn);
>         int i;
>         unsigned int crtcs_needed = 0;
> +       int crtcshift;
>
>         drmmode = xnfalloc(sizeof *drmmode);
>         drmmode->fd = fd;
> @@ -1444,8 +1445,9 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, int fd, int cpp)
>         }
>
>         xf86DrvMsg(pScrn->scrnIndex, X_INFO, "Initializing outputs ...\n");
> +       crtcshift = ffs(pNVEnt->assigned_crtcs ^ 0xffffffff) - 1;

Mario, any objections if I touch this up as

~pNVEnc->assigned_crtcs? XOR with ~0 is not a pattern I've seen a lot.

>         for (i = 0; i < drmmode->mode_res->count_connectors; i++)
> -               crtcs_needed += drmmode_output_init(pScrn, drmmode, i);
> +               crtcs_needed += drmmode_output_init(pScrn, drmmode, i, crtcshift);
>
>         xf86DrvMsg(pScrn->scrnIndex, X_INFO,
>                    "%d crtcs needed for screen.\n", crtcs_needed);
> --
> 1.9.1
>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Take shift in crtc positions for ZaphodHeads configs into account.
       [not found]     ` <CAKb7UvgWqo7HH+-fuO+FTSqrusotFcMoCG6Juc2J5ObM9QRpYQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2015-08-06 18:44       ` Mario Kleiner
  2015-11-15  7:41       ` Mario Kleiner
  1 sibling, 0 replies; 4+ messages in thread
From: Mario Kleiner @ 2015-08-06 18:44 UTC (permalink / raw)
  To: Ilia Mirkin
  Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	Ben Skeggs

On 08/06/2015 07:34 PM, Ilia Mirkin wrote:
> I don't understand this patch (what are all these masks? how are they
> used?), and don't want to invest the time required to do so.
>

I can't blame you for that :). The confused commit message reflects my 
confusion when i was trying to figure out how this (didn't) work on some 
few NVidia cards, while working on most of them ;) - Took me three 
separate attempts to find out the why, because the error messages from 
server and kernel send you in all kinds of random directions of what the 
cause of the failure could be.

Anyway. The kencoder->possible_crtcs mask encodes which hardware crtcs 
can go with the given kencoder. i'th bit set == i'th crtc usable for 
that encoder. The mask comes from nouveau-kms which gets it from the 
VBIOS DCB table. Most cards seem to just have 1 bits everywhere, so any 
crtc can go with any encoder. Some cards like the one in my Laptop have 
hard-wired encoder->crtc, so there can be 0 bits.

During crtc enumeration when initializing an X-Screen, the driver fills 
the x-screens xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn); 
crtc config array with enumerated crtc's which it gets from the kernel, 
always starting at slot 0 of the config->crtc[] array for each x-screen. 
On a single-x-screen setup, this means that gpu hardware crtc i ends in 
config->crtc[i], so config->crtc[i] corresponds to bit i in the 
possible_crtcs bitmask.

Some setup code inside the server assumes that this 1-1 correspondence 
between bits in possible_crtcs and config->crtc slots is always true.

On a ZaphodHeads setup this isn't the case, e.g., if you have a 
dual-head setup and assign one output and crtc to x-screen 0 and the 
other output and crtc to x-screen 1, then crtc 0 will end in 
config->crtc[0] of x-screen 0, and crtc 1 will end in config->crtc[0] of 
x-screen 1, as filling of the config->crtc[] arrays happens separately 
for each x-screen, always starting at slot 0.

So now you'd have hw crtc 1, which corresponds to bit 1 in 
kencoder->possible_crtcs ending in slot 0 of x-screen 1, instead of slot 
1 as would be the case on a non-Zaphod setup.

During init, nouveau calls the x-server function 
xf86InitialConfiguration(pScrn, TRUE), which calls xf86PickCrtcs(), 
which has this validation/matching code (hw/xfree86/modes/xf86Crtc.c)

     /*
      * Select a crtc for this output and
      * then attempt to configure the remaining
      * outputs
      */
     for (c = 0; c < config->num_crtc; c++) {
         if ((output->possible_crtcs & (1 << c)) == 0)
             continue;

         crtc = config->crtc[c];

-> It assumes bit c == slot c in config->crtc[c], therefore it fails to 
find a proper setup if that correspondence is broken on a ZaphodHeads 
setup. It matches the wrong bit position in the possible_crtc mask. If 
you get lucky (on many Nvidia cards) and the wrong bit position has a 1 
bit, then it works by accident. If you get unlucky like on my laptopt it 
bails out, aborting the server with the most confusing/misleading 
sequence of error messages in xorg.log and dmesg you could imagine.

So the patch tries to bit-shift the output->possible_crtcs mask when 
assigning from the kernels kencoder->possible_crtcs, to restore the 1-1 
correspondence between hw crtcs and slots in pScrn->config->crtc[] to 
make the servers matching code happy.

The crtcshift is the shift to apply to do this. pNVEnt->assigned_crtcs 
is a bit mask which marks already in use hw crtcs. Searching for the 
first 0 bit in that mask via that ffs() statement gives you the index of 
the first free hw crtc, the one which will end up in the 
pScrn->config->crtc[0] slot for the to-be-setup x-screen, or iow. how 
many bits you need to shift the output->possible_crtcs bitmask to 
restore a 1-1 correspondence from the perspective of the server.

On a single x-screen setup, that ffs() statment always gives you a 
crtcshift of 0 as pNVEnt->assigned_crtcs is all zero at server 
startup/regen, and that routine is called only one for a gpu, so the 
patch turns into a no-op for the regular case.

> However Mario is probably the sole serious user of ZaphodHeads, and if
> it fixes issues for him, probably fixes issues for others who try and
> give up with ZaphodHeads. Any objections if I just push this out?

Alex Deucher just reviewed and r-b'd the same patch for 
xf86-video-modesetting's ZaphodHeads implementation (on xorg-devel), so 
assuming he followed my convoluted commit message, that could count as a 
vote for the correctness of this one.

Atm. only nouveau and modesetting need the patch. intel and 
radeon/amdgpu and other ddx'en don't need it because their kms drivers 
always report a kencoder->possible_crtcs bitmask with as many 1 bits as 
there are hw crtcs - any encoder seems to go with any crtc. nouveau-kms 
seems to be the only one that does something clever with that bitmask, 
so needs a bit of special treatment.

>
> On Sat, Jun 27, 2015 at 8:33 PM, Mario Kleiner
> <mario.kleiner.de@gmail.com> wrote:
>> In multi-x-screen ZaphodHeads configurations, there isn't a
>> one-to-one mapping of kernel provided drmmode crtc index
>> to the index of the corresponding xf86Crtc inside the
>> xf86CrtcConfig crtc array anymore, ie. for kernel provided
>> drmmode->mode_res->crtcs[i], the i'th crtc won't correspond
>> to the xf86Crtc in the i'th slot of the x-screens xf86CrtcConfig
>> anymore, once ZaphodHeads has only selected a subset of all crtcs
>> of a graphics card for a given x-screen, instead of all crtcs.
>>
>> This breaks the mapping of bit positions in the bit masks returned
>> in kencoder->possible_crtcs and kencoder->possible_clones. A 1 bit
>> in position i of those masks allows use of the kernels i'th crtc for
>> the given kencoder. The X-Servers dix code checks those bit masks
>> for valid xf86Output -> xf86Crtc assignments, assuming that the i'th
>> slot xf86CrtcConfigPtr config->crtc[i] corresponds to bit i in the
>> xf86Output->possibe_crtcs bitmask, and bails if the bitmask doesn't
>> allow the specified assignment of crtc to output. If ZaphodHeads
>> breaks the assumption of bit i <-> crtc slot i this ends in failure.
>>
>> Take this shift of crtc index positions wrt. encoder bitmask bit
>> positions into account by bit-shifting positions accordingly when
>> assigning encoder->possible_crtcs to output->possible_crtcs, so
>> the proper indices match up again for validation by the dix.
>>
>> This problem wasn't apparent last year when testing the ZaphodHeads
>> support on some Kepler cards, as apparently the encoder->possible_crtcs
>> bitmasks returned for those cards by the kernel just had all 4
>> lsb bits set for all tested encoders/output, so each of the cards 4
>> crtcs could go with each output and things worked by chance.
>>
>> The current code breaks, e.g., on 2010 MacBookPro with nv50, where
>> one crtc is hardwired to the internal lvds panel, and one crtc
>> is hardwired to the external DP connector, resulting in a failure
>> where dual-display on single-x-screen works fine, but assigning
>> each output to a separate x-screen via ZaphodHeads fails due to
>> the mismatched encoder->possible_crtcs bitmasks.
>>
>> This patch fixes the problem.
>>
>> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
>> ---
>>   src/drmmode_display.c | 10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
>> index c30cb3a..3679482 100644
>> --- a/src/drmmode_display.c
>> +++ b/src/drmmode_display.c
>> @@ -1214,7 +1214,7 @@ drmmode_zaphod_match(ScrnInfoPtr pScrn, const char *s, char *output_name)
>>   }
>>
>>   static unsigned int
>> -drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num)
>> +drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num, int crtcshift)
>>   {
>>          NVPtr pNv = NVPTR(pScrn);
>>          xf86OutputPtr output;
>> @@ -1296,8 +1296,8 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num)
>>          output->subpixel_order = subpixel_conv_table[koutput->subpixel];
>>          output->driver_private = drmmode_output;
>>
>> -       output->possible_crtcs = kencoder->possible_crtcs;
>> -       output->possible_clones = kencoder->possible_clones;
>> +       output->possible_crtcs = kencoder->possible_crtcs >> crtcshift;
>> +       output->possible_clones = kencoder->possible_clones >> crtcshift;
>>
>>          output->interlaceAllowed = true;
>>          output->doubleScanAllowed = true;
>> @@ -1421,6 +1421,7 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, int fd, int cpp)
>>          NVEntPtr pNVEnt = NVEntPriv(pScrn);
>>          int i;
>>          unsigned int crtcs_needed = 0;
>> +       int crtcshift;
>>
>>          drmmode = xnfalloc(sizeof *drmmode);
>>          drmmode->fd = fd;
>> @@ -1444,8 +1445,9 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, int fd, int cpp)
>>          }
>>
>>          xf86DrvMsg(pScrn->scrnIndex, X_INFO, "Initializing outputs ...\n");
>> +       crtcshift = ffs(pNVEnt->assigned_crtcs ^ 0xffffffff) - 1;
>
> Mario, any objections if I touch this up as
>
> ~pNVEnc->assigned_crtcs? XOR with ~0 is not a pattern I've seen a lot.
>

Sure, fix it up, bit-wise not is much simpler. Thanks!
-mario


>>          for (i = 0; i < drmmode->mode_res->count_connectors; i++)
>> -               crtcs_needed += drmmode_output_init(pScrn, drmmode, i);
>> +               crtcs_needed += drmmode_output_init(pScrn, drmmode, i, crtcshift);
>>
>>          xf86DrvMsg(pScrn->scrnIndex, X_INFO,
>>                     "%d crtcs needed for screen.\n", crtcs_needed);
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Take shift in crtc positions for ZaphodHeads configs into account.
       [not found]     ` <CAKb7UvgWqo7HH+-fuO+FTSqrusotFcMoCG6Juc2J5ObM9QRpYQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2015-08-06 18:44       ` Mario Kleiner
@ 2015-11-15  7:41       ` Mario Kleiner
  1 sibling, 0 replies; 4+ messages in thread
From: Mario Kleiner @ 2015-11-15  7:41 UTC (permalink / raw)
  To: Ilia Mirkin
  Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	Ben Skeggs

Ping? Could we merge this patch? Nobody seems to have objections, it 
works since months on my test machines and an identical patch got 
reviewed and merged into xf86-video-modesetting, where it works without 
trouble, so it should be safe to apply.

thanks,
-mario

On 08/06/2015 07:34 PM, Ilia Mirkin wrote:
> I don't understand this patch (what are all these masks? how are they
> used?), and don't want to invest the time required to do so.
>
> However Mario is probably the sole serious user of ZaphodHeads, and if
> it fixes issues for him, probably fixes issues for others who try and
> give up with ZaphodHeads. Any objections if I just push this out?
>
> On Sat, Jun 27, 2015 at 8:33 PM, Mario Kleiner
> <mario.kleiner.de@gmail.com> wrote:
>> In multi-x-screen ZaphodHeads configurations, there isn't a
>> one-to-one mapping of kernel provided drmmode crtc index
>> to the index of the corresponding xf86Crtc inside the
>> xf86CrtcConfig crtc array anymore, ie. for kernel provided
>> drmmode->mode_res->crtcs[i], the i'th crtc won't correspond
>> to the xf86Crtc in the i'th slot of the x-screens xf86CrtcConfig
>> anymore, once ZaphodHeads has only selected a subset of all crtcs
>> of a graphics card for a given x-screen, instead of all crtcs.
>>
>> This breaks the mapping of bit positions in the bit masks returned
>> in kencoder->possible_crtcs and kencoder->possible_clones. A 1 bit
>> in position i of those masks allows use of the kernels i'th crtc for
>> the given kencoder. The X-Servers dix code checks those bit masks
>> for valid xf86Output -> xf86Crtc assignments, assuming that the i'th
>> slot xf86CrtcConfigPtr config->crtc[i] corresponds to bit i in the
>> xf86Output->possibe_crtcs bitmask, and bails if the bitmask doesn't
>> allow the specified assignment of crtc to output. If ZaphodHeads
>> breaks the assumption of bit i <-> crtc slot i this ends in failure.
>>
>> Take this shift of crtc index positions wrt. encoder bitmask bit
>> positions into account by bit-shifting positions accordingly when
>> assigning encoder->possible_crtcs to output->possible_crtcs, so
>> the proper indices match up again for validation by the dix.
>>
>> This problem wasn't apparent last year when testing the ZaphodHeads
>> support on some Kepler cards, as apparently the encoder->possible_crtcs
>> bitmasks returned for those cards by the kernel just had all 4
>> lsb bits set for all tested encoders/output, so each of the cards 4
>> crtcs could go with each output and things worked by chance.
>>
>> The current code breaks, e.g., on 2010 MacBookPro with nv50, where
>> one crtc is hardwired to the internal lvds panel, and one crtc
>> is hardwired to the external DP connector, resulting in a failure
>> where dual-display on single-x-screen works fine, but assigning
>> each output to a separate x-screen via ZaphodHeads fails due to
>> the mismatched encoder->possible_crtcs bitmasks.
>>
>> This patch fixes the problem.
>>
>> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
>> ---
>>   src/drmmode_display.c | 10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
>> index c30cb3a..3679482 100644
>> --- a/src/drmmode_display.c
>> +++ b/src/drmmode_display.c
>> @@ -1214,7 +1214,7 @@ drmmode_zaphod_match(ScrnInfoPtr pScrn, const char *s, char *output_name)
>>   }
>>
>>   static unsigned int
>> -drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num)
>> +drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num, int crtcshift)
>>   {
>>          NVPtr pNv = NVPTR(pScrn);
>>          xf86OutputPtr output;
>> @@ -1296,8 +1296,8 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num)
>>          output->subpixel_order = subpixel_conv_table[koutput->subpixel];
>>          output->driver_private = drmmode_output;
>>
>> -       output->possible_crtcs = kencoder->possible_crtcs;
>> -       output->possible_clones = kencoder->possible_clones;
>> +       output->possible_crtcs = kencoder->possible_crtcs >> crtcshift;
>> +       output->possible_clones = kencoder->possible_clones >> crtcshift;
>>
>>          output->interlaceAllowed = true;
>>          output->doubleScanAllowed = true;
>> @@ -1421,6 +1421,7 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, int fd, int cpp)
>>          NVEntPtr pNVEnt = NVEntPriv(pScrn);
>>          int i;
>>          unsigned int crtcs_needed = 0;
>> +       int crtcshift;
>>
>>          drmmode = xnfalloc(sizeof *drmmode);
>>          drmmode->fd = fd;
>> @@ -1444,8 +1445,9 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, int fd, int cpp)
>>          }
>>
>>          xf86DrvMsg(pScrn->scrnIndex, X_INFO, "Initializing outputs ...\n");
>> +       crtcshift = ffs(pNVEnt->assigned_crtcs ^ 0xffffffff) - 1;
>
> Mario, any objections if I touch this up as
>
> ~pNVEnc->assigned_crtcs? XOR with ~0 is not a pattern I've seen a lot.
>
>>          for (i = 0; i < drmmode->mode_res->count_connectors; i++)
>> -               crtcs_needed += drmmode_output_init(pScrn, drmmode, i);
>> +               crtcs_needed += drmmode_output_init(pScrn, drmmode, i, crtcshift);
>>
>>          xf86DrvMsg(pScrn->scrnIndex, X_INFO,
>>                     "%d crtcs needed for screen.\n", crtcs_needed);
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-11-15  7:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-28  0:33 [PATCH] Take shift in crtc positions for ZaphodHeads configs into account Mario Kleiner
     [not found] ` <1435451629-17959-1-git-send-email-mario.kleiner.de-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-08-06 17:34   ` Ilia Mirkin
     [not found]     ` <CAKb7UvgWqo7HH+-fuO+FTSqrusotFcMoCG6Juc2J5ObM9QRpYQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-08-06 18:44       ` Mario Kleiner
2015-11-15  7:41       ` Mario Kleiner

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.