[PATCH xserver] randr: Do not check the screen size bound for gpu screens

[Hans de Goede]

Hi,

On 20-05-16 07:00, Nikhil Mahale wrote:
> For gpu screen, CrtcSet set/adjust the master screen size along
> mode in following callstack -
>
>   ProcRRSetCrtcConfig()
>     |
>     -> RRCrtcSet()
>         |
>         -> rrCheckPixmapBounding()
>             |
>             -> pScrPriv->rrScreenSetSize()
>
> Checking screen size bound for gpus screen cause some configurations
> to fails, e.g
>
>   $ xrandr --output eDP --mode 1920x1080 --pos 0x0 --output HDMI \
>   --mode 2560x1440 --pos 0x0
>
>   Here xrandr utility first sets screen size to 2560x1440 which
>   gets resized to 1920x1080 on RRSetCrtcConfig request for eDP,
>   and then RRSetCrtcConfig request for HDMI fails because
>   of failure of screen bound check.

Hmm, but one has the same problem when not using clone mode,
then the master screen size must be made big enough to hold
both crtc-s, so how come this has never been a problem then ?

I've a feeling that in that case we end up doing things in
finer grained steps. What if you do:

$ xrandr --output eDP --mode 1920x1080 --pos 0x0
$ xrandr --output HDMI --mode 2560x1440 --pos 0x0

? Does that work ?  If that works, which I would expect it will,
then this should be fixed in the xrandr utility instead IMHO.

Just circumventing the screen size check for slave outputs
seem to go against the xrandr-1.2 spec to me.

Also I guess one can reproduce the same problem without using
slave-outputs, in which case your patch will not help.


Regards,

Hans


>
> Signed-off-by: Nikhil Mahale <nmahale at nvidia.com>
> ---
>  randr/rrcrtc.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
> index 566a3db..82db9a8 100644
> --- a/randr/rrcrtc.c
> +++ b/randr/rrcrtc.c
> @@ -1176,27 +1176,20 @@ ProcRRSetCrtcConfig(ClientPtr client)
>
>  #ifdef RANDR_12_INTERFACE
>          /*
> +         * For gpu screen, CrtcSet set/adjust the master screen size along
> +         * with mode.
> +         *
>           * Check screen size bounds if the DDX provides a 1.2 interface
>           * for setting screen size. Else, assume the CrtcSet sets
>           * the size along with the mode. If the driver supports transforms,
>           * then it must allow crtcs to display a subset of the screen, so
>           * only do this check for drivers without transform support.
>           */
> -        if (pScrPriv->rrScreenSetSize && !crtc->transforms) {
> +        if (!pScreen->isGPU && pScrPriv->rrScreenSetSize && !crtc->transforms) {
>              int source_width;
>              int source_height;
>              PictTransform transform;
>              struct pixman_f_transform f_transform, f_inverse;
> -            int width, height;
> -
> -            if (pScreen->isGPU) {
> -                width = pScreen->current_master->width;
> -                height = pScreen->current_master->height;
> -            }
> -            else {
> -                width = pScreen->width;
> -                height = pScreen->height;
> -            }
>
>              RRTransformCompute(stuff->x, stuff->y,
>                                 mode->mode.width, mode->mode.height,
> @@ -1206,13 +1199,13 @@ ProcRRSetCrtcConfig(ClientPtr client)
>
>              RRModeGetScanoutSize(mode, &transform, &source_width,
>                                   &source_height);
> -            if (stuff->x + source_width > width) {
> +            if (stuff->x + source_width > pScreen->width) {
>                  client->errorValue = stuff->x;
>                  free(outputs);
>                  return BadValue;
>              }
>
> -            if (stuff->y + source_height > height) {
> +            if (stuff->y + source_height > pScreen->height) {
>                  client->errorValue = stuff->y;
>                  free(outputs);
>                  return BadValue;
>