Re: RFC: xserver release planning

2024-02-06 Thread Michel Dänzer
On 2024-02-06 12:19, Enrico Weigelt, metux IT consult wrote:
> On 06.02.24 10:07, Michel Dänzer wrote:
> 
>>> #4 xorg master and xwayland have massively diverged, pretty much a fork
>>
>> Not sure what you mean by that. If you're looking at the xwayland-2x.y 
>> release branches,
> 
> Yes, e.g. xwayland-23.2 is the one used by debian (unstable).
> 
>> those drop code specific to other DDXen, since that serves no purpose for 
>> standalone Xwayland releases.
> 
> Doesn't make much sense to me.

No problem, you can just ignore it.

> IIRC, one just builds with a config that only enables Xwayland and leaves off 
> the
> others. Note that it's not just a bunch of files removed - there're even 
> features
> removed that happen to be ununsed by xwayland.

Only Xwayland can be built & installed from a standalone Xwayland release. 
Anything which is only used by other DDXen would be dead weight in the Xwayland 
release tarballs.


> The diff is huge (even w/o the removed files) and git history has
> diverged for almost 200 commits,

This is explained above.


> w/o any clear point-of-fork.

?

xwayland-23.2 forked from master commit 94deed272cbd ("xwayland: Use sensible 
defaults for rootful size"). The only commits on xwayland-* branches which 
aren't on master are those removing stuff specific to other DDXen. There's no 
divergence.


> master is still at 21.1.* while xwayland is at 23.2.* - this really
> doesn't add up to me.

It's irrelevant that the versions aren't consistent, since xserver & Xwayland 
are now released separately.


>>> * gitlab: add xserver-23.2 milestone (realign w/ xwayland)
>>
>> It's 2024 already. There's no point in aligning with Xwayland, which is 
>> released separately anyway.
> 
> Well, I don't think it's a good idea to split that, in the long run.

That ship sailed over 3 years ago.


-- 
Earthling Michel Dänzer|  https://redhat.com
Libre software enthusiast  | Mesa and Xwayland developer



Re: RFC: xserver release planning

2024-02-06 Thread Michel Dänzer
On 2024-02-05 21:10, Enrico Weigelt, metux IT consult wrote:
> 
> #4 xorg master and xwayland have massively diverged, pretty much a fork

Not sure what you mean by that. If you're looking at the xwayland-2x.y release 
branches, those drop code specific to other DDXen, since that serves no purpose 
for standalone Xwayland releases. There's no divergence though, Xwayland is 
being developed on the master branch and its release branches start from 
current master at the time.


> * gitlab: add xserver-23.2 milestone (realign w/ xwayland)

It's 2024 already. There's no point in aligning with Xwayland, which is 
released separately anyway.


> * work trough differences between master and xwayland branch and try
>   to align them to each other (at some point in the future they should
>   be pretty much equal

Per above, nothing to do here.


-- 
Earthling Michel Dänzer|  https://redhat.com
Libre software enthusiast  | Mesa and Xwayland developer



Re: Plan for an Xwayland 23.2 release

2023-07-07 Thread Michel Dänzer
On 7/7/23 14:52, Olivier Fourdan wrote:
> Hi all,
> 
> On Thu, Jun 15, 2023 at 11:01 AM Olivier Fourdan  wrote:
>>
>> Hi all,
>>
>> There are changes in Xwayland that I would like to see landed as part
>> of another Xwayland 23.2 release this year, as those could not make it
>> on time for 23.1, namely libEI support (now that libEI 1.0 was
>> released), and possibly  a few other changes:
>>
>>  - Add EI support
>>https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/975
>>  - Implement wp_tearing_control_v1
>>https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/665
>>
> 
> Both merge requests have now landed, so I'd like to start the process.
> 
> For this purpose, I've created a new milestone "Xwayland-23.2.0":
> https://gitlab.freedesktop.org/xorg/xserver/-/milestones/16
> 
> And pushed the  preparation work in a merge request:
> https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1132
> 
> (** Not ** to be merged until the branch for xwayland 23.2 is created!)
> 
> As for the schedule, I would like to propose the following:
> 
>  * July 19th, 2023: Xwayland 23.2.0 RC1
>  * August 2nd, 2023: Xwayland 23.2.0 RC2
>  * August 16th, 2023: Xwayland 23.2.0

Sounds great to me. Hopefully we can merge 
https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1131 before 
cutting the branch, assuming no major issues with it come up in the meantime.


-- 
Earthling Michel Dänzer|  https://redhat.com
Libre software enthusiast  | Mesa and Xwayland developer



Re: SCM_RIGHTS: XTrans vs ssh forwarding with MIT-SHM clients

2023-01-16 Thread Michel Dänzer
On 1/16/23 06:31, Jeremy Huddleston Sequoia wrote:
> 
> How should this work?  Why hasn't this been reported as an issue on other 
> platforms?  This all seems pretty platform agnostic, so I'd expect this to be 
> an issue on other platforms as well.  Is it not?  If not, why not?

ComputeLocalClient attempts to detect SSH clients and treats them as non-local. 
Maybe this isn't working on macos for some reason?


-- 
Earthling Michel Dänzer|  https://redhat.com
Libre software enthusiast  | Mesa and Xwayland developer



Re: Questions about XPresent

2022-10-18 Thread Michel Dänzer
On 2022-10-18 16:14, Po Lu wrote:
> Michel Dänzer  writes:
> 
>> The problem with this is that there's no explicit transfer of
>> ownership of the window pixmap between the compositor and other
>> entities. So the compositor may end up reading from a stale window
>> pixmap after another presentation has already flipped in a new pixmap,
>> and the client may already be drawing a new frame to the pixmap used
>> by the compositor, resulting in visual artifacts.
> 
> If the compositor issues a Composite request, then by the time the
> request arrives on the X server, won't the pixmap resource point to the
> data of the pixmap that is currently busy?  Or are you saying that the X
> server keeps references to pixmap data that it has sent IdleNotify
> events for, and may use those pixmaps in response to a future request
> from the compositor?

I'm thinking of this kind of scenario with a direct rendering compositor:

1. Compositor retrieves window pixmap, creates OpenGL texture object for it.
2. Client sends PresentPixmap request, which results in replacing the window 
pixmap.
3. Client starts drawing new frame to the old window pixmap buffer.
4. Compositor samples from texture object created in step 1.


I did actually try this a while ago, and saw artifacts due to this issue.


-- 
Earthling Michel Dänzer|  https://redhat.com
Libre software enthusiast  | Mesa and Xwayland developer



Re: Questions about XPresent

2022-10-18 Thread Michel Dänzer
On 2022-10-07 09:09, Po Lu wrote:
> 
> The documentation for PresentPixmap says:
> 
>   If 'options' contains PresentOptionAsync, and the 'target-msc'
>   is less than or equal to the current msc for 'window', then
>   the operation will be performed as soon as possible, not
>   necessarily waiting for the next vertical blank interval.
> 
> What does the "necessarily" here mean?  That the presentation might
> still wait for vblank, even if PresentOptionAsync is specified, and the
> target MSC is less than the MSC of the window?

I think so.

However, AFAICT the xserver Present code will always execute the presentation 
ASAP in this case, never wait for vblank first.


> What will a following SyncSetCounter do?  Will the counter always be set
> to the new value after the presentation happens and the window damage is
> sent to the compositor?

You mean if the presentation waited for vblank first? If so, I don't see any 
such implicit synchronization between PresentPixmap and SyncSetCounter (or any 
other request).


> My second question is about drawing to the window after presentation
> happens.  When direct presentation becomes impossible (by, for example,
> the Wayland client attaching a subsurface), then the results of any
> previously made presentation must be overwritten by the following
> Composite requests.  What happens if a Composite request is made,
> targeting a window whose contents have been previously specified with
> PresentPixmap, especially if the pixmap has been flipped and not copied?
> 
> Do I have to present an empty pixmap, wait for any PresentIdleNotify
> for the previously attached pixmap, or what?

You don't need to do anything special. The effect of PresentPixmap is 
essentially the same as XCopyArea as far as the destination window contents are 
concerned, regardless of how the presentation is effectively performed.


> That brings me to the third question.  Wouldn't a good optimization be
> to "flip" the presented pixmap, if possible, into the contents of the
> window pixmap if the window were to be redirected?  Compositing managers
> would then be able to read from the window immediately after
> presentation, without needing the contents of the presented pixmap to be
> copied to the window pixmap first.

The problem with this is that there's no explicit transfer of ownership of the 
window pixmap between the compositor and other entities. So the compositor may 
end up reading from a stale window pixmap after another presentation has 
already flipped in a new pixmap, and the client may already be drawing a new 
frame to the pixmap used by the compositor, resulting in visual artifacts.

(Xwayland can do essentially what you're describing, since it has an explicit 
transfer of ownership via Wayland)


-- 
Earthling Michel Dänzer|  https://redhat.com
Libre software enthusiast  | Mesa and Xwayland developer



Re: Enabling gitlab CI across all the X.Org repos

2022-08-03 Thread Michel Dänzer
On 2022-08-03 00:19, Alan Coopersmith wrote:
> 
> Hopefully having CI setup will help make meson conversions easier.
> (For those doing such conversions, see the xproto or libXvMC .gitlab-ci.yml
>  for examples on running both builds during the transition period.)

FWIW, xserver also had this when it still had autotools support, that can still 
be inspected in the Git history as well.


-- 
Earthling Michel Dänzer|  https://redhat.com
Libre software enthusiast  | Mesa and Xwayland developer



Re: xserver: Branch 'master' - 3 commits

2022-06-21 Thread Michel Dänzer
On 2022-06-21 05:37, Jeremy Sequoia wrote:
> I reverted

Thanks.

Note that "make dist" is still broken on server-22.1-branch: 
https://gitlab.freedesktop.org/xorg/xserver/-/jobs/24330392 (I don't personally 
care about this, but the 22.1 branch maintainer might :)


> and created a pull request to track getting this in once the pipelines are 
> updated:
> 
>     https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/913 

I commented there how the CI failure could be solved.


>> On Jun 20, 2022, at 17:41, Jeremy Sequoia > <mailto:jerem...@apple.com>> wrote:
>>> On Jun 20, 2022, at 02:09, Michel Dänzer >> <mailto:michel.daen...@mailbox.org>> wrote:
>>>
>>> [0] That said, if the Xquartz build could be tested as part of the GitLab 
>>> CI pipeline, that could be useful for you as well.
>>
>> Could you point me at documentation for setting that up?

I don't know of any documentation about how to do builds for macOS in GitLab 
CI, which might be the main challenge here. I'd suggest joining the 
#freedesktop channel on OFTC IRC and asking the gstreamer folks there how 
they're doing it. Or maybe take a look at the macos job definitions in 
https://gitlab.freedesktop.org/gstreamer/cerbero/-/blob/main/.gitlab-ci.yml .

FWIW, the general GitLab CI/CD documentation is at 
https://docs.gitlab.com/ee/ci/ and the documentation for the CI templates we're 
using at https://freedesktop.pages.freedesktop.org/ci-templates/ . I'm afraid 
this may be overwhelming at first though, it certainly was for me. :)


-- 
Earthling Michel Dänzer|  https://redhat.com
Libre software enthusiast  | Mesa and Xwayland developer


Re: xserver: Branch 'master' - 3 commits

2022-06-20 Thread Michel Dänzer


Hi Jeremy,


good to see you working on Xquartz again!


We are generally not pushing xserver changes directly to the main repository 
anymore, but using GitLab merge requests instead. While you are mostly free to 
ignore this for changes which affect Xquartz only[0], bumping the minimum meson 
version requirement does not affect only Xquartz. This change broke the GitLab 
CI pipeline on master & server-22.1-branch:

https://gitlab.freedesktop.org/xorg/xserver/-/pipelines/616828
https://gitlab.freedesktop.org/xorg/xserver/-/pipelines/616834 (Looks like 
another commit broke make dist as well)

Until this is resolved, it won't be possible to merge any GitLab MRs (which 
don't resolve the issue) to these branches.


[0] That said, if the Xquartz build could be tested as part of the GitLab CI 
pipeline, that could be useful for you as well.


On 2022-06-20 07:21, GitLab Mirror wrote:
>  
> commit 0a27f96d1d0e474b308be982fa7069d3ae0d9892
> Author: Jeremy Huddleston Sequoia 
> Date:   Sun Jun 19 22:18:16 2022 -0700
> 
> meson: Bump requirement to meson-0.50.0
> 
> WARNING: Project specifies a minimum meson_version '>= 0.47.0' but uses 
> features which were added in newer versions:
>  * 0.50.0: {'install arg in configure_file'}
> 
> Signed-off-by: Jeremy Huddleston Sequoia 
> 
> diff --git a/meson.build b/meson.build
> index db1d63f3e..7f9330107 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -4,7 +4,7 @@ project('xserver', 'c',
>  'c_std=gnu99',
>  ],
>  version: '21.1.99.1',
> -meson_version: '>= 0.47.0',
> +meson_version: '>= 0.50.0',
>  )
>  release_date = '2021-07-05'
>  


-- 
Earthling Michel Dänzer|  https://redhat.com
Libre software enthusiast  | Mesa and Xwayland developer


Re: X server new numbering scheme

2021-12-09 Thread Michel Dänzer
On 2021-12-06 22:36, Matthieu Herrb wrote:
> Hi,
> 
> I don't understand why the 1.20.x -> 21.x.y change was needed, but I
> can accept it.
> 
> That said, why try to keep VENDOR_RELEASE in the form 1.21.x.y « for
> backward compatibility » according to comments in meson.build and
> confiugure.ac (in 21.1 branch) ?
> 
> This causes XORG_VERSION_SNAP to be set to a non zero value in 21.1.1
> and causes the following text to be displayed on startup, even though
> this is a relased version.
> 
> """
> This is a pre-release version of the X server from The X.Org
> Foundation.
> It is not supported in any way.
> Bugs may be filed in the bugzilla at http://bugs.freedesktop.org/.
> Select the "xorg" product for bugs you find in this release.
> Before reporting bugs in pre-release versions please check the
> latest version in the X.Org Foundation git repository.
> See http://wiki.x.org/wiki/GitPage for git access instructions.
> """
> 
> Some may make jokes out of the "It is not supported in any way."
> sentence. Are relases any more supported  ?
> 
> I'd like to either submit a merge request to remove the PRE_RELEASE
> test completely from xf86Init.c, or just drop the "compatibililty"
> right shift of the version. I'm typeing this from an X server build
> with
> 
> VENDOR_RELEASE =  (((21) * 1000) + ((1) * 10) + ((1) * 1000) + 0)
> 
> and have found no issue yet. What was the need for this "backward
> compatibility" ?

Mainly for clients doing things like 
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8999#note_799189 I 
think.


-- 
Earthling Michel Dänzer|  https://redhat.com
Libre software enthusiast  | Mesa and Xwayland developer


Re: Xwayland 21.1.0 release schedule proposal

2021-02-03 Thread Michel Dänzer

On 2021-01-28 6:52 p.m., Michel Dänzer wrote:


Per my plan discussed at 
https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/582, I'm 
proposing the following schedule:


* February 3rd: Create xwayland-21.1 branch


Done: https://gitlab.freedesktop.org/xorg/xserver/-/commits/xwayland-21.1



Any remaining fixes will be cherry picked from the master branch.



Note: All changes which are applicable to the master branch must be 
merged there first, then backported to this branch.



--
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Xwayland 21.1.0 release schedule proposal

2021-01-28 Thread Michel Dänzer


Per my plan discussed at 
https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/582, I'm 
proposing the following schedule:


* February 3rd: Create xwayland-21.1 branch
* February 17th: First release candidate
* March 3rd: Second release candidate
* March 17th: Final 21.1.0 release

There may be more release candidates (or just one), depending on the 
feedback we get from testing.


Let me know if you think any of these should be moved up or back.


I've created an "xwayland-21.1.0" milestone: 
https://gitlab.freedesktop.org/xorg/xserver/-/milestones/11


This milestone should be set on issues / MRs which need to be addressed 
or at least considered before the 21.1.0 release.



--
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


[RFC] Making Xwayland-only releases

2020-12-18 Thread Michel Dänzer


https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/582 has a 
proposal for Xwayland-only releases, including a proof of concept for 
how it could be done.



In order to avoid scattering the discussion between there and here, may 
I kindly request any feedback to be provided on the merge request page.



--
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH xserver] WIP : Add support for variable refresh rate in modesetting DDX

2020-06-15 Thread Michel Dänzer

Forgot something:


On 2020-06-10 2:05 p.m., pichika.uday.ki...@intel.com wrote:
> 
>  mode change 100644 => 100755 hw/xfree86/drivers/modesetting/driver.c
>  mode change 100644 => 100755 hw/xfree86/drivers/modesetting/driver.h
>  mode change 100644 => 100755 hw/xfree86/drivers/modesetting/drmmode_display.c
>  mode change 100644 => 100755 hw/xfree86/drivers/modesetting/drmmode_display.h
>  mode change 100644 => 100755 hw/xfree86/drivers/modesetting/present.c

Looks like you've set the executable permission bits for these files in
your xserver tree. Please fix that.


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH xserver] WIP : Add support for variable refresh rate in modesetting DDX

2020-06-11 Thread Michel Dänzer

Thanks for your patch.

These days, we're using GitLab merge requests for submitting and
reviewing xserver patches:
https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests

Anyway, while we're here, some early feedback:

First of all, it would be better to port xf86-video-amdgpu commits
2d18b37159ed "Check last flip window instead of screen root before
flipping" & ef8fbe33b7d9 "Wrap change/delete window property request
handlers" as separate commits for the modesetting driver as well.


On 2020-06-10 2:05 p.m., pichika.uday.ki...@intel.com wrote:
> 
> Kernel Changes to support this feature is under development.

This is a bit confusing as is, since the amdgpu kernel driver already
has everything needed. Presumably you're referring to the i915 driver?
I'm not sure that really needs to be mentioned here.


> What is Tested ?
> Verified with DOTA2, Xonotic gaming apps, private glxapp runs in
> Fullscreen mode. When these apps are running in fullscreen mode,
> able to call the set_vrr on all the CRTCs with VRR_ENABLED property.

This could be simplified to something like

 Tested with DOTA2, Xonotic and custom GLX apps.


> @@ -3023,6 +3100,9 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr 
> drmmode, drmModeResPtr mode_r
>  }
>  /* work out the possible clones later */
>  output->possible_clones = 0;
> +props = drmModeObjectGetProperties(drmmode->fd,
> +   drmmode_output->output_id,
> +   DRM_MODE_OBJECT_CONNECTOR);
>  
>  if (ms->atomic_modeset) {
>  if (!drmmode_prop_info_copy(drmmode_output->props_connector,
> @@ -3048,6 +3128,7 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr 
> drmmode, drmModeResPtr mode_r
>  RRPostPendingProperties(output->randr_output);
>  }
>  }
> +ms->is_connector_vrr_capable = 
> drmmode_connector_check_vrr_capable(drmmode->fd, props, "VRR_CAPABLE");
>  
>  return 1;
>  

drmModeFreeObjectProperties(props);

is needed at the end of drmmode_output_init, or it leaks the resources
referenced by props.


> @@ -295,11 +311,17 @@ ms_present_check_flip(RRCrtcPtr crtc,
>  ScreenPtr screen = window->drawable.pScreen;
>  ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
>  modesettingPtr ms = modesettingPTR(scrn);
> +Bool ret;
>  
>  if (ms->drmmode.sprites_visible > 0)
>  return FALSE;
>  
> -return ms_present_check_unflip(crtc, window, pixmap, sync_flip, reason);
> +if(ret = ms_present_check_unflip(crtc, window, pixmap, sync_flip, 
> reason))
> +{
> +ms->flip_window = window;
> +}
> +
> +return ret;

This doesn't match the style of the existing code very well. I'd write
it as:

if (!ms_present_check_unflip(...))
return FALSE;

ms->flip_window = window;
return TRUE;
}


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: cherry-pick candidate for a future 1.20.9 release

2020-04-15 Thread Michel Dänzer
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 2020-04-14 7:46 p.m., Matthieu Herrb wrote:
> On Tue, Apr 14, 2020 at 11:12:37AM +0200, Michel Dänzer wrote:
>> -BEGIN PGP SIGNED MESSAGE- Hash: SHA1
>>
>> On 2020-04-13 7:52 p.m., Matthieu Herrb wrote:
>>> Hi,
>>>
>>> I'd like to nominate 364d64981549544213e2bca8de6ff8a5b2b5a69e
>>> as a commit to cherry-pick into a future release of the 1.20
>>> branch.
>>>
>>> Without this commit, SDL 1.2 games in full screen get FocusOut
>>> events every couple of seconds, and this is causing in some of
>>> them (burgerspace is an example that was used to show me the
>>> issue) it will pause the game, rendering it un-playable.
>>>
>>> |commit 364d64981549544213e2bca8de6ff8a5b2b5a69e |Author:
>>> Samuel Thibault  |Date:   Tue Oct
>>> 30 18:43:51 2018 +0100 | |dix: do not send focus event when
>>> grab actually does not change
>>>
>>> Thanks in advance.
>>
>> Can you create a GitLab merge request for server-1.20-branch?
>
> Hi,
>
> Done. Hope it's right as I've not used gitlab much yet...
>
> https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/422

Thanks!

Just two nits:

Use "git cherry-pick -x" for backports in general, so the commit log
contains a reference to the original commit on master. (You can fix up
the commit log and force-push to your personal branch)

Check the "allow [...] with access to the target branch" box in the MR
settings. (This isn't currently needed for this MR, but it often is in
other cases, so it's better to always check it when creating an MR)


- -- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
-BEGIN PGP SIGNATURE-

iF0EARECAB0WIQSwn681vpFFIZgJURRaga+OatuyAAUCXpbX1gAKCRBaga+Oatuy
AJXLAJ9VtX8MwKA77b++mG93BgVlopovNQCcDvgicaK9bgFctwI0LFVzeZODJ+k=
=wpx9
-END PGP SIGNATURE-
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: cherry-pick candidate for a future 1.20.9 release

2020-04-14 Thread Michel Dänzer
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 2020-04-13 7:52 p.m., Matthieu Herrb wrote:
> Hi,
>
> I'd like to nominate 364d64981549544213e2bca8de6ff8a5b2b5a69e as a
> commit to cherry-pick into a future release of the 1.20 branch.
>
> Without this commit, SDL 1.2 games in full screen get FocusOut
> events every couple of seconds, and this is causing in some of
> them (burgerspace is an example that was used to show me the issue)
> it will pause the game, rendering it un-playable.
>
> |commit 364d64981549544213e2bca8de6ff8a5b2b5a69e |Author: Samuel
> Thibault  |Date:   Tue Oct 30
> 18:43:51 2018 +0100 | |dix: do not send focus event when grab
> actually does not change
>
> Thanks in advance.

Can you create a GitLab merge request for server-1.20-branch?


- -- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
-BEGIN PGP SIGNATURE-

iF0EARECAB0WIQSwn681vpFFIZgJURRaga+OatuyAAUCXpV+fAAKCRBaga+Oatuy
AMfMAJ9903MfuS7Um5tPQI46aj9CLvLJ3ACgtk4gRiPlIePizJz0sdLbd1591hM=
=Rfae
-END PGP SIGNATURE-
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: gitlab.fd.o financial situation and impact on services

2020-04-06 Thread Michel Dänzer
On 2020-04-06 6:34 p.m., Rob Clark wrote:
>
> The ideal thing would be to be able to click any jobs that we want to
> run, say "arm64_a630_gles31", and for gitlab to realize that it needs
> to automatically trigger dependencies of that job (meson-arm64, and
> arm_build+arm_test).  But not sure if that is a thing gitlab can do.

Not that I know of. The dependency handling is still pretty rudimentary
in general.


> Triggering just the first container jobs and having everything from
> there run automatically would be ok if the current rules to filter out
> unneeded jobs still apply, ie. a panfrost change isn't triggering
> freedreno CI jobs and visa versa.  But tbh I don't understand enough
> of what that MR is doing to understand if that is what it does.  (It
> was suggested on IRC that this is probably what it does.)

It is. Filtered jobs don't exist at all in the pipeline, so they can't
be triggered by any means. :)


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: [Mesa-dev] [Intel-gfx] gitlab.fd.o financial situation and impact on services

2020-04-04 Thread Michel Dänzer
On 2020-03-01 6:46 a.m., Marek Olšák wrote:
> For Mesa, we could run CI only when Marge pushes, so that it's a strictly
> pre-merge CI.

Thanks for the suggestion! I implemented something like this for Mesa:

https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4432


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: RFC: Minimum meson version for xserver 20

2020-03-30 Thread Michel Dänzer
On 2020-03-30 5:23 p.m., Adam Jackson wrote:
> Does anyone have strong opinions on this? I would really like to bump
> to at least 0.49 for the position-independent executable support. If
> not that, 0.47 gives us 'feature' support for build options, which
> addresses the "should we enable this by default or not" question in a
> consistent way.

Even stock Debian stable has 0.49.2 (backports has 0.52.1), so 0.49
seems fair game.


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: Plumbing explicit synchronization through the Linux ecosystem

2020-03-20 Thread Michel Dänzer
On 2020-03-19 8:54 p.m., Marek Olšák wrote:
> On Thu., Mar. 19, 2020, 06:51 Daniel Vetter, 
> wrote:
>> 
>> Yeah, this is entirely about the programming model visible to
>> userspace. There shouldn't be any impact on the driver's choice of
>> a top vs. bottom of the gpu pipeline used for synchronization,
>> that's entirely up to what you're hw/driver/scheduler can pull
>> off.
>> 
>> Doing a full gfx pipeline flush for shared buffers, when your hw
>> can do be, sounds like an issue to me that's not related to this
>> here at all. It might be intertwined with amdgpu's special
>> interpretation of dma_resv fences though, no idea. We might need to
>> revamp all that. But for a userspace client that does nothing fancy
>> (no multiple render buffer targets in one bo, or vk style "I write
>> to everything all the time, perhaps" stuff) there should be 0 perf
>> difference between implicit sync through dma_resv and explicit sync
>> through sync_file/syncobj/dma_fence directly.
>> 
>> If there is I'd consider that a bit a driver bug.
> 
> Last time I checked, there was no fence sync in gnome shell and
> compiz after an app passes a buffer to it.

They are not required (though encouraged) to do that.


> So drivers have to invent hacks to work around it and decrease
> performance. It's not a driver bug.
> 
> Implicit sync really means that apps and compositors don't sync, so
> the driver has to guess when it should sync.

Making implicit sync work correctly is ultimately the kernel driver's
responsibility. It sounds like radeonsi is having to work around the
amdgpu/radeon kernel driver(s) not fully living up to this responsibility.


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: [Mesa-dev] Plumbing explicit synchronization through the Linux ecosystem

2020-03-19 Thread Michel Dänzer
On 2020-03-17 6:21 p.m., Lucas Stach wrote:
> That's one of the issues with implicit sync that explicit may solve: 
> a single client taking way too much time to render something can 
> block the whole pipeline up until the display flip. With explicit 
> sync the compositor can just decide to use the last client buffer if 
> the latest buffer isn't ready by some deadline.

FWIW, the compositor can do this with implicit sync as well, by polling
a dma-buf fd for the buffer. (Currently, it has to poll for writable,
because waiting for the exclusive fence only isn't enough with amdgpu)


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: Plumbing explicit synchronization through the Linux ecosystem

2020-03-17 Thread Michel Dänzer
On 2020-03-16 7:33 p.m., Marek Olšák wrote:
> On Mon, Mar 16, 2020 at 5:57 AM Michel Dänzer  wrote:
>> On 2020-03-16 4:50 a.m., Marek Olšák wrote:
>>> The synchronization works because the Mesa driver waits for idle (drains
>>> the GFX pipeline) at the end of command buffers and there is only 1
>>> graphics queue, so everything is ordered.
>>>
>>> The GFX pipeline runs asynchronously to the command buffer, meaning the
>>> command buffer only starts draws and doesn't wait for completion. If the
>>> Mesa driver didn't wait at the end of the command buffer, the command
>>> buffer would finish and a different process could start execution of its
>>> own command buffer while shaders of the previous process are still
>> running.
>>>
>>> If the Mesa driver submits a command buffer internally (because it's
>> full),
>>> it doesn't wait, so the GFX pipeline doesn't notice that a command buffer
>>> ended and a new one started.
>>>
>>> The waiting at the end of command buffers happens only when the flush is
>>> external (Swap buffers, glFlush).
>>>
>>> It's a performance problem, because the GFX queue is blocked until the
>> GFX
>>> pipeline is drained at the end of every frame at least.
>>>
>>> So explicit fences for SwapBuffers would help.
>>
>> Not sure what difference it would make, since the same thing needs to be
>> done for explicit fences as well, doesn't it?
> 
> No. Explicit fences don't require userspace to wait for idle in the command
> buffer. Fences are signalled when the last draw is complete and caches are
> flushed. Before that happens, any command buffer that is not dependent on
> the fence can start execution. There is never a need for the GPU to be idle
> if there is enough independent work to do.

I don't think explicit fences in the context of this discussion imply
using that different fence signalling mechanism though. My understanding
is that the API proposed by Jason allows implicit fences to be used as
explicit ones and vice versa, so presumably they have to use the same
signalling mechanism.


Anyway, maybe the different fence signalling mechanism you describe
could be used by the amdgpu kernel driver in general, then Mesa could
drop the waits for idle and get the benefits with implicit sync as well?


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: [Mesa-dev] Plumbing explicit synchronization through the Linux ecosystem

2020-03-16 Thread Michel Dänzer
On 2020-03-16 4:50 a.m., Marek Olšák wrote:
> The synchronization works because the Mesa driver waits for idle (drains
> the GFX pipeline) at the end of command buffers and there is only 1
> graphics queue, so everything is ordered.
> 
> The GFX pipeline runs asynchronously to the command buffer, meaning the
> command buffer only starts draws and doesn't wait for completion. If the
> Mesa driver didn't wait at the end of the command buffer, the command
> buffer would finish and a different process could start execution of its
> own command buffer while shaders of the previous process are still running.
> 
> If the Mesa driver submits a command buffer internally (because it's full),
> it doesn't wait, so the GFX pipeline doesn't notice that a command buffer
> ended and a new one started.
> 
> The waiting at the end of command buffers happens only when the flush is
> external (Swap buffers, glFlush).
> 
> It's a performance problem, because the GFX queue is blocked until the GFX
> pipeline is drained at the end of every frame at least.
> 
> So explicit fences for SwapBuffers would help.

Not sure what difference it would make, since the same thing needs to be
done for explicit fences as well, doesn't it?


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: [Mesa-dev] [Intel-gfx] gitlab.fd.o financial situation and impact on services

2020-03-01 Thread Michel Dänzer
On 2020-02-29 8:46 p.m., Nicolas Dufresne wrote:
> Le samedi 29 février 2020 à 19:14 +0100, Timur Kristóf a écrit :
>>
>> 1. I think we should completely disable running the CI on MRs which are
>> marked WIP. Speaking from personal experience, I usually make a lot of
>> changes to my MRs before they are merged, so it is a waste of CI
>> resources.

Interesting idea, do you want to create an MR implementing it?


> In the mean time, you can help by taking the habit to use:
> 
>   git push -o ci.skip

That breaks Marge Bot.


> Notably, we would like to get rid of the post merge CI, as in a rebase
> flow like we have in GStreamer, it's a really minor risk.

That should be pretty easy, see Mesa and
https://docs.gitlab.com/ce/ci/variables/predefined_variables.html.
Something like this should work:

  rules:
- if: '$CI_PROJECT_NAMESPACE != "gstreamer"'
  when: never

This is another interesting idea we could consider for Mesa as well. It
would however require (mostly) banning direct pushes to the main repository.


>> 2. Maybe we could take this one step further and only allow the CI to
>> be only triggered manually instead of automatically on every push.

That would again break Marge Bot.


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: [Mesa-dev] [Intel-gfx] gitlab.fd.o financial situation and impact on services

2020-02-28 Thread Michel Dänzer
On 2020-02-28 12:02 p.m., Erik Faye-Lund wrote:
> On Fri, 2020-02-28 at 10:43 +, Daniel Stone wrote:
>> On Fri, 28 Feb 2020 at 10:06, Erik Faye-Lund
>>  wrote:
>>> On Fri, 2020-02-28 at 11:40 +0200, Lionel Landwerlin wrote:
>>>> Yeah, changes on vulkan drivers or backend compilers should be
>>>> fairly
>>>> sandboxed.
>>>>
>>>> We also have tools that only work for intel stuff, that should
>>>> never
>>>> trigger anything on other people's HW.
>>>>
>>>> Could something be worked out using the tags?
>>>
>>> I think so! We have the pre-defined environment variable
>>> CI_MERGE_REQUEST_LABELS, and we can do variable conditions:
>>>
>>> https://docs.gitlab.com/ee/ci/yaml/#onlyvariablesexceptvariables
>>>
>>> That sounds like a pretty neat middle-ground to me. I just hope
>>> that
>>> new pipelines are triggered if new labels are added, because not
>>> everyone is allowed to set labels, and sometimes people forget...
>>
>> There's also this which is somewhat more robust:
>> https://gitlab.freedesktop.org/mesa/mesa/merge_requests/2569
> 
> I'm not sure it's more robust, but yeah that a useful tool too.
> 
> The reason I'm skeptical about the robustness is that we'll miss
> testing if this misses a path.

Surely missing a path will be less likely / often to happen compared to
an MR missing a label. (Users which aren't members of the project can't
even set labels for an MR)


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: [Mesa-dev] [Intel-gfx] gitlab.fd.o financial situation and impact on services

2020-02-28 Thread Michel Dänzer
On 2020-02-28 10:28 a.m., Erik Faye-Lund wrote:
> 
> We could also do stuff like reducing the amount of tests we run on each
> commit, and punt some testing to a per-weekend test-run or someting
> like that. We don't *need* to know about every problem up front, just
> the stuff that's about to be released, really. The other stuff is just
> nice to have. If it's too expensive, I would say drop it.

I don't agree that pre-merge testing is just nice to have. A problem
which is only caught after it lands in mainline has a much bigger impact
than one which is already caught earlier.


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: Understanding of Composition call flow in XServer

2020-02-28 Thread Michel Dänzer
On 2020-02-27 1:35 p.m., uday kiran pichika wrote:
> Hello,
> 
> I'm new to X Server and working on a new feature development. As part of my
> job, am going through the composition call flow in X. To study this, i have
> added/modified  xf86DrvMsg logs under *xfree86/drivers/modesetting* to
> understand the output and crtc initialization and DRM commits and launched
> X with the below command on virtual terminal.
> 
> sudo startx -- -logverbose 6
> 
> But none of the logs are being printed as part of Xorg.n.log file except
> these
> [  3158.316] (II) intel(0): Uday --- fbdevhw.c : fbdev_open
> [  3158.317] (==) intel(0): Uday - Default visual is TrueColor
> [  3158.318] (II) intel(0): Uday --- xf86RandR12.c : xf86RandR12Init
> [  3158.327] (II) intel(0): Uday --- Setting screen physical size to 508 x
> 285

This is shows Xorg using the intel driver, not modesetting.


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: Composite redraw speedup?

2020-02-13 Thread Michel Dänzer
On 2020-02-12 6:42 p.m., Carsten Haitzler wrote:
> On Wed, 12 Feb 2020 16:45:50 +0100 Michel Dänzer  said:
> 
>> On 2020-02-12 4:34 p.m., Carsten Haitzler wrote:
>>> On Wed, 12 Feb 2020 15:36:06 +0100 Michel Dänzer  said:
>>>
>>>> On 2020-02-12 2:06 p.m., Carsten Haitzler wrote:
>>>>> On Wed, 12 Feb 2020 13:23:28 +0200 Pekka Paalanen 
>>>>> said:
>>>>>
>>>>>> On Wed, 12 Feb 2020 11:07:56 +
>>>>>> Carsten Haitzler  wrote:
>>>>>>
>>>>>>> On Wed, 12 Feb 2020 12:40:15 +0200 Pekka Paalanen 
>>>>>>> said:
>>>>>>>
>>>>>>>> On Wed, 12 Feb 2020 10:21:02 +
>>>>>>>> Carsten Haitzler (The Rasterman)  wrote:
>>>>>>>>   
>>>>>>>>> even better - if the /dev/dri/card0
>>>>>>>>> device exists, dlopen libdrm and get some symbols from it and ... use
>>>>>>>>> it to request the drm device sent you vsync events so you can use the
>>>>>>>>> vsync interrupt as your frame event. this will be another fd to listen
>>>>>>>>> on in select() and of course you can turn this vblank event stream on
>>>>>>>>> and off.  
>>>>>>>>
>>>>>>>> Please don't. Talk to the X server instead.  
>>>>>>>
>>>>>>> and what vsync events does the xserver provide? 
>>>>>>
>>>>>> You don't want vsync events. You have no idea what they
>>>>>> correspond to, or even if you opened the right device.
>>>>>>
>>>>>> https://gitlab.freedesktop.org/xorg/proto/xorgproto/blob/master/presentproto.txt
>>>>>
>>>>> I wrote the drm support before the present extension existed. The drm
>>>>> path is  easy to support - only open if a single card exists (if multiple
>>>>> - don't do it and fall back to timer based animation) and you can filter
>>>>> for multiple screens as you get events for all screens. Yes - you end up
>>>>> syncing with a single chosen screen if you filter for just one of the
>>>>> vblank events, but it's better than using the system clock.
>>>>
>>>> You only get an event for the CRTC you ask for in the
>>>> DRM_IOCTL_WAIT_VBLANK ioctl. How do yo know which CRTC to pick?
>>>
>>> drmVBlank vbl;
>>> vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
>>> vbl.request.sequence = 1;
>>> vbl.request.signal = 0;
>>> drmWaitVBlank(drm_fd, );
>>>
>>> has worked a charm for maybe give or take close to a decade (as i said -
>>> before present existed).
>>
>> That selects the first CRTC. It'll work most of the time, except when
>> the first CRTC isn't enabled for some reason.
> 
> I've seen it select multiple crtc's as I get multiple vblank events with the
> same frame number... (thus filter out the dups to only get one).

Sounds like something's wrong.

Try setting vbl.request.signal to a unique (e.g. monotonically
increasing) value for each drmWaitVBlank call. The value is replicated
in the user_data field of the corresponding event. If you get multiple
events with the same user_data value, that indicates a kernel bug.


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: Composite redraw speedup?

2020-02-12 Thread Michel Dänzer
On 2020-02-12 4:34 p.m., Carsten Haitzler wrote:
> On Wed, 12 Feb 2020 15:36:06 +0100 Michel Dänzer  said:
> 
>> On 2020-02-12 2:06 p.m., Carsten Haitzler wrote:
>>> On Wed, 12 Feb 2020 13:23:28 +0200 Pekka Paalanen 
>>> said:
>>>
>>>> On Wed, 12 Feb 2020 11:07:56 +
>>>> Carsten Haitzler  wrote:
>>>>
>>>>> On Wed, 12 Feb 2020 12:40:15 +0200 Pekka Paalanen 
>>>>> said:
>>>>>
>>>>>> On Wed, 12 Feb 2020 10:21:02 +
>>>>>> Carsten Haitzler (The Rasterman)  wrote:
>>>>>>   
>>>>>>> even better - if the /dev/dri/card0
>>>>>>> device exists, dlopen libdrm and get some symbols from it and ... use
>>>>>>> it to request the drm device sent you vsync events so you can use the
>>>>>>> vsync interrupt as your frame event. this will be another fd to listen
>>>>>>> on in select() and of course you can turn this vblank event stream on
>>>>>>> and off.  
>>>>>>
>>>>>> Please don't. Talk to the X server instead.  
>>>>>
>>>>> and what vsync events does the xserver provide? 
>>>>
>>>> You don't want vsync events. You have no idea what they
>>>> correspond to, or even if you opened the right device.
>>>>
>>>> https://gitlab.freedesktop.org/xorg/proto/xorgproto/blob/master/presentproto.txt
>>>
>>> I wrote the drm support before the present extension existed. The drm
>>> path is  easy to support - only open if a single card exists (if multiple -
>>> don't do it and fall back to timer based animation) and you can filter for
>>> multiple screens as you get events for all screens. Yes - you end up syncing
>>> with a single chosen screen if you filter for just one of the vblank events,
>>> but it's better than using the system clock.
>>
>> You only get an event for the CRTC you ask for in the
>> DRM_IOCTL_WAIT_VBLANK ioctl. How do yo know which CRTC to pick?
> 
> drmVBlank vbl;
> vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
> vbl.request.sequence = 1;
> vbl.request.signal = 0;
> drmWaitVBlank(drm_fd, );
> 
> has worked a charm for maybe give or take close to a decade (as i said - 
> before
> present existed).

That selects the first CRTC. It'll work most of the time, except when
the first CRTC isn't enabled for some reason.


>>> I have found x present XPresentNotifyMSC() to be unreliable where the
>>> drm back-door above is far more reliable. For example - on my amdgpu driver
>>> here it just refuses to produce any events (yes - extension is there), [...]
>>
>> I haven't seen any such reports, so I'm not aware of such issues with
>> xf86-video-amdgpu.
> 
> i'm staring at it not working :) admittedly it's a slightly older rx550 with
> amdgpu on an aarch64 host... :) i did play with xpresent in the early days it
> came out and found it to be iffy and stuck to the solution we already had
> (above) which is more reliable.

A proper issue report with all the needed information will be needed to
do anything about this.


To be clear, even the Present extension is really too low level to be
used directly for this by a compositor (unless it also uses it directly
for the actual presentation). The DRM vblank ioctl even more so, it's
for the display server.

A compositor which uses GLX/EGL for presentation should use
corresponding GLX/EGL (extension) APIs for this.


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: Composite redraw speedup?

2020-02-12 Thread Michel Dänzer
On 2020-02-12 2:06 p.m., Carsten Haitzler wrote:
> On Wed, 12 Feb 2020 13:23:28 +0200 Pekka Paalanen  said:
> 
>> On Wed, 12 Feb 2020 11:07:56 +
>> Carsten Haitzler  wrote:
>>
>>> On Wed, 12 Feb 2020 12:40:15 +0200 Pekka Paalanen 
>>> said:
>>>
>>>> On Wed, 12 Feb 2020 10:21:02 +
>>>> Carsten Haitzler (The Rasterman)  wrote:
>>>>   
>>>>> even better - if the /dev/dri/card0
>>>>> device exists, dlopen libdrm and get some symbols from it and ... use
>>>>> it to request the drm device sent you vsync events so you can use the
>>>>> vsync interrupt as your frame event. this will be another fd to listen
>>>>> on in select() and of course you can turn this vblank event stream on
>>>>> and off.  
>>>>
>>>> Please don't. Talk to the X server instead.  
>>>
>>> and what vsync events does the xserver provide? 
>>
>> You don't want vsync events. You have no idea what they
>> correspond to, or even if you opened the right device.
>>
>> https://gitlab.freedesktop.org/xorg/proto/xorgproto/blob/master/presentproto.txt
> 
> I wrote the drm support before the present extension existed. The drm
> path is  easy to support - only open if a single card exists (if multiple -
> don't do it and fall back to timer based animation) and you can filter for
> multiple screens as you get events for all screens. Yes - you end up syncing
> with a single chosen screen if you filter for just one of the vblank events,
> but it's better than using the system clock.

You only get an event for the CRTC you ask for in the
DRM_IOCTL_WAIT_VBLANK ioctl. How do yo know which CRTC to pick?


> I have found x present XPresentNotifyMSC() to be unreliable where the
> drm back-door above is far more reliable. For example - on my amdgpu driver
> here it just refuses to produce any events (yes - extension is there), [...]

I haven't seen any such reports, so I'm not aware of such issues with
xf86-video-amdgpu.


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: Overlay Text issue with Xorg modesetting driver

2020-01-20 Thread Michel Dänzer
On 2020-01-17 9:49 a.m., Shubham Kumar Jha wrote:
> Hi,
> 
> I have attached an example which displays text and background rectangle When 
> this piece of code is run with Intel as default XORG driver everything works 
> fine both text and rectangle are being displayed,whereas when i switch to the 
> Modesetting driver only the background rectangle is seen and text is not 
> being displayed.
> 
> I tried running XShmqueryversion with modesetting driver , it returned value 
> of pixmap as True, so it means shared pixmaps should be supported.I have 
> verified this behavior on Ubuntu 16.04.
> 
> Kindly let me know if this is the expected behavior of modesetting driver, if 
> not then how to go ahead with resolving this.

It could be a bug in glamor, but why are you using an SHM pixmap for
this anyway? AFAICT you're only drawing to it with Xft, not directly
with the CPU on the client side. So you could use a normal pixmap and
copy from that to the window with XCopyArea. That'll also work on setups
which actually don't support SHM pixmaps, e.g. Xwayland or Xorg with
EXA, and as a bonus should even perform better.


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


Re: xserver release process

2019-10-09 Thread Michel Dänzer

On 2019-10-08 6:28 p.m., Adam Jackson wrote:

I gave a brief lightning talk about this at XDC Montreal [1], and
nobody seemed to object, but here's a recap for those who weren't
present or have other ideas or preferences.

In short, releases need to happen, and we have CI, so let's just pop a
release out on scheduled dates assuming CI passes. Six months seems to
be a pretty reasonable cadence for xserver major releases as new
feature development has tailed off a bit. Likewise for stable branches,
if there's been changes to the branch and it's been (say) two weeks
since the last release on that branch, and CI passes, automatically do
a new release. I intend to make this _entirely_ automatic, with a robot
doing the actual commits and release uploads.


I like the idea of doing automatic release snapshots / candidates, but I 
think that's a bit too aggressive for actual releases at this point, at 
least on the stable branch. E.g. the recent 32-bit ABI breakage on the 
1.20 branch would likely have made it into a 1.20.y release with this 
scheme.




Also, let's adopt the Mesa "last two digits of the year" version
scheme, it makes it easy to see how old your software is at a glance.
We'll need to be slightly careful about this to ensure we don't make
the (protocol) release number go crazy, so the scheme looks something
like this (underscores for clarity):

- xserver 1.20.5 → 1_20_05_000
- xserver 19.0.0 → 1_20_19_000
- xserver 19.0.1 → 1_20_19_001
- xserver 19.1.0 → 1_20_19_100
- xserver 20.0.0 → 1_20_20_000


And then 1_21_*_* as of 21.x.y? Either way, sounds good.


--
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: modular/release.sh issue

2019-09-23 Thread Michel Dänzer
On 2019-09-20 2:51 p.m., Martin-Éric Racine wrote:
> Howdy!
> 
> I finally got around migrating my local Git tree to GitLab and was
> about to push a new release of xf86-video-geode, but ran into this
> issue with modular/release.sh:
> 
> https://paste.debian.net/1101728/
> 
> The development files are all within my home directory, and ownership
> and permissions are -R to my username, so I'm not sure of what would
> cause these:
> 
> cp: cannot create regular file '../../.INSTALL.tmp': Permission denied

I think this is a harmless issue in autotools and can be ignored. This
happens at some point during make distcheck, but (with
xf86-video-amdgpu/ati) it works fine at another point, and the file is
included in the generated tarballs.


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] modesetting: Update props for dynamically added outputs

2019-08-21 Thread Michel Dänzer
On 2019-08-20 6:01 p.m., Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Dynamically added outputs should have their properties
> properly updated as well. Otherwise we're left with an output
> with many of its propeties not exposed.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  hw/xfree86/drivers/modesetting/drmmode_display.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
> b/hw/xfree86/drivers/modesetting/drmmode_display.c
> index e0d7cfb5c708..f621df52f3f7 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> @@ -3003,8 +3003,14 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr 
> drmmode, drmModeResPtr mode_r
>  "DPMS");
>  }
>  
> -if (dynamic)
> +if (dynamic) {
>  output->randr_output = RROutputCreate(xf86ScrnToScreen(pScrn), 
> output->name, strlen(output->name), output);
> +if (output->randr_output) {
> +drmmode_output_create_resources(output);
> +RRPostPendingProperties(output->randr_output);
> +}
> +}
> +
>  return 1;
>  
>   out_free_encoders:
> 

Reviewed-by: Michel Dänzer 


P.S. xserver patches are now being reviewed as GitLab merge requests:
https://gitlab.freedesktop.org/xorg/xserver/merge_requests

-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [Mesa-dev] All-black X windows on glamor with etnaviv

2019-07-04 Thread Michel Dänzer
On 2019-07-04 5:28 a.m., Jiaxun Yang wrote:
> Hi folks,
> 
> We had some reports that glamor is not working correctly with etnaviv
> mesa driver for long time. Previously Lukas Hartmann wad proposed a
> patch [1] replacing linear rendering buffer with a tiled buffer and it
> do works but giving scrambled output. We just dig the issue deeper and
> managed to let it give out a normal output.
> 
> Since Vivante hardware doesn't actually support tiled surface/texture,
> once the driver accpet a imported buffer or called to create a resource,
> it will make a shadow tiled buffer, and convert the content in tiled
> buffer to the content it linear buffer when  etna_flush_resource in
> mesa/src/gallium/drivers/etnaviv/etnaviv_clear_blit.c is called.
> However, it was never been called with glamor. By contrast other
> applications like kmscube or weston using EGL GLES have correct behavior.
> 
> Our solution is just flush the buffer everytime , it's not a elegant
> way, but it works. Since GC1000 is only giving OpenGL 1.4 rather than
> 2.1 as minimal requirement of glamor. We also have to force glamor work
> with GLES.
> 
> Any idea if we can solve this issue correctly?

I suspect etna_resource_get_handle needs to check for
PIPE_HANDLE_USAGE_EXPLICIT_FLUSH. If it's not set, the caller will not
call the flush_resource hook for this resource. This needs to be
recorded in struct etna_resource. Then in etna_flush, if this was
recorded for the resource backing any of the cbufs in the current
pipe_framebuffer_state, etna_flush_resource needs to be called for that
resource.

It might be possible to optimize this somewhat, e.g. maybe etna_flush
only needs to do this when it's called with certain PIPE_FLUSH_* flags
(not) set corresponding to glFlush, but not for other internal flushes.


-- 
Earthling Michel Dänzer   |  https://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: xf86-video-intel: 2 commits - src/sna/sna_blt.c src/sna/sna_display.c

2019-03-02 Thread Michel Dänzer
On 2019-03-01 7:22 p.m., GitLab Mirror wrote:
>  src/sna/sna_blt.c |7 +--
>  src/sna/sna_display.c |   20 ++--
>  2 files changed, 11 insertions(+), 16 deletions(-)
> 
> New commits:
> commit 6afed33b2d673d88674f0c76efe500ae414e8e1b
> Author: Ville Syrjälä 
> Date:   Fri Mar 1 11:26:07 2019 +
> 
> sna: Switch back to hwcursor on the next cursor update
> 
> Once we've switched to using the swcursor (possibly
> due to the cursor ioctl failing) we currently keep
> using the swcursor until the modeset.
> 
> That's not particularly great as the swcursor has several
> issues. Apart from the (presumably expected) flicker,
> the cursor also tends to leave horrible trails behind
> around dri2/3 windows (happens with tearfree at least).

Sounds like the intel driver is allowing DRI2/Present page flipping
while there's an SW cursor? That cannot work correctly, as the SW cursor
image is missing in the newly flipped pixmap, and the SW cursor code
will restore stale background contents to it when the cursor has to be
drawn again.


-- 
Earthling Michel Dänzer   |  https://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: xserver 1.20.4 maintenance release

2019-02-22 Thread Michel Dänzer
On 2019-02-20 10:47 p.m., Adam Jackson wrote:
> Hey, I need one of these, so I've got a merge request posted with some
> bugfixes from master backported:
> 
> https://gitlab.freedesktop.org/xorg/xserver/merge_requests/125
> 
> If there's anything in there you hate, or anything not in there that
> should be, give a shout.

I just wanted to cherry pick another Present fix, but ended up having to
pull in the GitLab CI changes as well:
https://gitlab.freedesktop.org/xorg/xserver/merge_requests/128


> I'd like to push this out sometime this week.

It's great that you're making a 1.20.4 release!

FWIW, might be worth pulling in
https://gitlab.freedesktop.org/xorg/xserver/merge_requests/127 as well,
so you can use make distcheck.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [Mesa-dev] RFC - libglvnd and GLXVND vendor enumeration to facilitate GLX multi-vendor PRIME GPU offload

2019-02-12 Thread Michel Dänzer
On 2019-02-11 5:18 p.m., Andy Ritger wrote:
> On Mon, Feb 11, 2019 at 12:09:26PM +0100, Michel Dänzer wrote:
>> On 2019-02-08 11:43 p.m., Kyle Brenneman wrote:
>>>
>>> Also, is Mesa the only client-side vendor library that works with the
>>> Xorg GLX module? I vaguely remember that there was at least one other
>>> driver that did, but I don't remember the details anymore.
>>
>> AFAIK, the amdgpu-pro OpenGL driver can work with the Xorg GLX module
>> (or its own forked version of it).
> 
> Maybe the amdgpu-pro OpenGL driver uses a fork of the Xorg GLX module
> (or sets the "GlxVendorLibrary" X configuration option?), but it doesn't
> look to me like the in-tree Xorg GLX module could report anything other
> than "mesa" for GLX_VENDOR_NAMES_EXT, without custom user configuration.
> 
> GLX_VENDOR_NAMES_EXT, which client-side glvnd uses to pick the
> libGLX_${vendor}.so to load, is implemented in the Xorg GLX module
> with this:
> 
>   xserver/glx/glxcmds.c:__glXDisp_QueryServerString():
> 
> case GLX_VENDOR_NAMES_EXT:
> if (pGlxScreen->glvnd) {
> ptr = pGlxScreen->glvnd;
> break;
> }
> 
> pGlxScreen->glvnd appears to be assigned here, defaulting to "mesa", 
> though allowing an xorg.conf override via the "GlxVendorLibrary" option:
> 
>   xserver/glx/glxdri2.c:__glXDRIscreenProbe():
> 
> xf86ProcessOptions(pScrn->scrnIndex, pScrn->options, options);
> glvnd = xf86GetOptValString(options, GLXOPT_VENDOR_LIBRARY);
> if (glvnd)
> screen->base.glvnd = xnfstrdup(glvnd);
> free(options);
> 
> if (!screen->base.glvnd)
> screen->base.glvnd = strdup("mesa");
> 
> And swrast unconditionally sets pGlxScreen->glvnd to "mesa":
> 
>   xserver/glx/glxdriswrast.c:__glXDRIscreenProbe():
> 
> screen->base.glvnd = strdup("mesa");
> 
> Is there more to this that I'm missing?

I don't think so, I suspect we were just assuming slightly different
definitions of "works". :)


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [Mesa-dev] RFC - libglvnd and GLXVND vendor enumeration to facilitate GLX multi-vendor PRIME GPU offload

2019-02-11 Thread Michel Dänzer
On 2019-02-08 11:43 p.m., Kyle Brenneman wrote:
> 
> Also, is Mesa the only client-side vendor library that works with the
> Xorg GLX module? I vaguely remember that there was at least one other
> driver that did, but I don't remember the details anymore.

AFAIK, the amdgpu-pro OpenGL driver can work with the Xorg GLX module
(or its own forked version of it).


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: Patch for glibc 2.25+ in xserver config/udev.c

2018-11-28 Thread Michel Dänzer
On 2018-11-28 5:12 a.m., Manoj Gupta wrote:
> Hello all,
> 
> if there are no objections to this patch, can someone merge it?

Adam merged it last week:

https://gitlab.freedesktop.org/xorg/xserver/commit/82f8cf8990009f6cac567814dd6b7fd41cfad82d


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: DRM leases + X = SW only OpenGL acceleration on child X server

2018-09-24 Thread Michel Dänzer
On 2018-09-24 12:04 p.m., Raimonds Cicans wrote:
> On 24.09.2018 12:17, Michel Dänzer wrote:
>> On 2018-09-23 11:18 p.m., Raimonds Cicans wrote:
>>> Hi!
>>>
>>> I am playing with new "DRM leases" feature.
>>> I am trying to implement single video card multi-seat.
>>>
>>> Questions:
>>>
>>> 1) is HW accelerated OpenGL possible at all on child X server?
>>>
>>> 2) if answer for first question is "yes", then what can cause following
>>> behaviour?
>>>
>>> DISPLAY=:100 glxgears # Main X server. Works without problem
>>>
>>> DISPLAY=:101 glxgears # Child X server. Get following errors
>>> libGL error: failed to authenticate magic 1
>>> libGL error: failed to load driver: radeonsi
>>
>> Please attach both Xorg log files and the output of
>>
>>  LIBGL_DEBUG=verbose DISPLAY=:101 glxgears
>>
>>
> 
> All files attached.

Looks like drmAuthMagic returns an error for the leased FD in the Xorg
process.

Keith, was this working for you?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: DRM leases + X = SW only OpenGL acceleration on child X server

2018-09-24 Thread Michel Dänzer
On 2018-09-23 11:18 p.m., Raimonds Cicans wrote:
> Hi!
> 
> I am playing with new "DRM leases" feature.
> I am trying to implement single video card multi-seat.
> 
> Questions:
> 
> 1) is HW accelerated OpenGL possible at all on child X server?
> 
> 2) if answer for first question is "yes", then what can cause following
> behaviour?
> 
> DISPLAY=:100 glxgears # Main X server. Works without problem
> 
> DISPLAY=:101 glxgears # Child X server. Get following errors
> libGL error: failed to authenticate magic 1
> libGL error: failed to load driver: radeonsi

Please attach both Xorg log files and the output of

 LIBGL_DEBUG=verbose DISPLAY=:101 glxgears


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [Pixman] [BUG] SIGSEGV in sse2_fill

2018-09-10 Thread Michel Dänzer
On 2018-09-10 12:01 p.m., Frédéric Fauberteau wrote:
> Le 2018-09-10 09:26, Michel Dänzer a écrit :
>> On 2018-09-10 8:22 a.m., Frédéric Fauberteau wrote:
>>> Le 2018-09-01 15:16, Michel Dänzer a écrit :
>>>> On 2018-09-01 9:22 a.m., Frédéric Fauberteau wrote:
>>>>>
>>>>> [  5770.134] (EE) RADEON(0): Failed to make prime FD for handle: 22
>>>>> [  5770.134] (EE) RADEON(0): Failed to create textured screen.
>>>>
>>>> This is the problem. Please try current xf86-video-ati Git master,
>>>> specifically
>>>> https://gitlab.freedesktop.org/xorg/driver/xf86-video-ati/commit/db28d35ce9fd07a2a4703f3df0633d4c8291ff9b
>>>>
>>>>
>>>> could help for this.
>>>
>>> I just tested this commit and xorg server now starts without
>>> segfaulting.
>>>
>>> I am from a remote host and I don't know if the display is correct. But
>>> the log file shows an error:
>>>
>>> [688070.735] (II) Initializing extension DRI2
>>> [688070.737] (II) RADEON(0): Setting screen physical size to 783 x 277
>>> [688070.923] failed to add FB for modeset
>>> [688070.923] (WW) RADEON(0): Failed to set mode on CRTC 0
>>> [688070.954] failed to add FB for modeset
>>> [688070.954] (WW) RADEON(0): Failed to set mode on CRTC 1
>>> [688070.955] (EE) RADEON(0): Failed to enable any CRTC
>>
>> Please try the Git master branch, which will soon become the 18.1
>> release. If there's still a problem with that, please provide the
>> corresponding full Xorg log file.
> 
> Please find the Xorg.0.log produced after upgrading the driver to
> de88ea2755611bdcb18d91d8234d2ab5be8ff2e9:

Thanks, the rest of the log file looks fine.

The above warning and error messages indicate that drmModeAddFB fails.
Since you got the "Failed to make prime FD for handle" error message
from glamor with the 18.0.1 driver, there might still be an issue
related to the GEM handle of the front buffer.

I've never seen this before, so I suspect it could be an issue specific
to NetBSD's KMS support. Can you poke around a bit to see why
drmModeAddFB fails? E.g., what are the parameters passed to it by
radeon_fb_create, and what error does it return?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [Pixman] [BUG] SIGSEGV in sse2_fill

2018-09-10 Thread Michel Dänzer
On 2018-09-10 8:22 a.m., Frédéric Fauberteau wrote:
> Le 2018-09-01 15:16, Michel Dänzer a écrit :
>> On 2018-09-01 9:22 a.m., Frédéric Fauberteau wrote:
>>>
>>> [  5770.134] (EE) RADEON(0): Failed to make prime FD for handle: 22
>>> [  5770.134] (EE) RADEON(0): Failed to create textured screen.
>>
>> This is the problem. Please try current xf86-video-ati Git master,
>> specifically
>> https://gitlab.freedesktop.org/xorg/driver/xf86-video-ati/commit/db28d35ce9fd07a2a4703f3df0633d4c8291ff9b
>>
>> could help for this.
> 
> I just tested this commit and xorg server now starts without segfaulting.
> 
> I am from a remote host and I don't know if the display is correct. But
> the log file shows an error:
> 
> [688070.735] (II) Initializing extension DRI2
> [688070.737] (II) RADEON(0): Setting screen physical size to 783 x 277
> [688070.923] failed to add FB for modeset
> [688070.923] (WW) RADEON(0): Failed to set mode on CRTC 0
> [688070.954] failed to add FB for modeset
> [688070.954] (WW) RADEON(0): Failed to set mode on CRTC 1
> [688070.955] (EE) RADEON(0): Failed to enable any CRTC

Please try the Git master branch, which will soon become the 18.1
release. If there's still a problem with that, please provide the
corresponding full Xorg log file.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver v2] xwayland: Remove xwl_present_window from privates on cleanup

2018-09-05 Thread Michel Dänzer
On 2018-09-05 10:35 a.m., Olivier Fourdan wrote:
> Xwayland's `xwl_destroy_window()` invokes `xwl_present_cleanup()`
> before the common `DestroyWindow()`.
> 
> But then `DestroyWindow()` calls `present_destroy_window()` which will
> possibly end up in `xwl_present_abort_vblank()` which will try to access
> data that was previously freed by `xwl_present_cleanup()`:
> 
>   Invalid read of size 8
>  at 0x434184: xwl_present_abort_vblank (xwayland-present.c:378)
>  by 0x53785B: present_wnmd_abort_vblank (present_wnmd.c:651)
>  by 0x53695A: present_free_window_vblank (present_screen.c:87)
>  by 0x53695A: present_destroy_window (present_screen.c:152)
>  by 0x42A90D: xwl_destroy_window (xwayland.c:653)
>  by 0x584298: compDestroyWindow (compwindow.c:613)
>  by 0x53CEE3: damageDestroyWindow (damage.c:1570)
>  by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326)
>  by 0x46F7F6: FreeWindowResources (window.c:1031)
>  by 0x472847: DeleteWindow (window.c:1099)
>  by 0x46B54C: doFreeResource (resource.c:880)
>  by 0x46C706: FreeClientResources (resource.c:1146)
>  by 0x446ADE: CloseDownClient (dispatch.c:3473)
>Address 0x182abde0 is 80 bytes inside a block of size 112 free'd
>  at 0x4C2FDAC: free (vg_replace_malloc.c:530)
>  by 0x42A937: xwl_destroy_window (xwayland.c:647)
>  by 0x584298: compDestroyWindow (compwindow.c:613)
>  by 0x53CEE3: damageDestroyWindow (damage.c:1570)
>  by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326)
>  by 0x46F7F6: FreeWindowResources (window.c:1031)
>  by 0x472847: DeleteWindow (window.c:1099)
>  by 0x46B54C: doFreeResource (resource.c:880)
>  by 0x46C706: FreeClientResources (resource.c:1146)
>  by 0x446ADE: CloseDownClient (dispatch.c:3473)
>  by 0x446DA5: ProcKillClient (dispatch.c:3279)
>  by 0x4476AF: Dispatch (dispatch.c:479)
>Block was alloc'd at
>  at 0x4C30B06: calloc (vg_replace_malloc.c:711)
>  by 0x433F46: xwl_present_window_get_priv (xwayland-present.c:54)
>  by 0x434228: xwl_present_get_crtc (xwayland-present.c:302)
>  by 0x539728: proc_present_query_capabilities (present_request.c:227)
>  by 0x4476AF: Dispatch (dispatch.c:479)
>  by 0x44B5B5: dix_main (main.c:276)
>  by 0x75F611A: (below main) (libc-start.c:308)
> 
> This is because `xwl_present_cleanup()` frees the memory but does not
> remove it from the window's privates, and `xwl_present_abort_vblank()`
> will still find it and hence try to access that freed memory...
> 
> Clear `xwl_present_cleanup()` after `DestroyWindow()` so that
> `xwl_present_abort_vblank()` can still access valid memory before it's
> freed.

This last paragraph doesn't seem to match the rest of the patch.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] glamor: Add support for exporting depth 16 pixmaps.

2018-09-05 Thread Michel Dänzer
On 2018-09-05 6:00 a.m., Eric Anholt wrote:
> With a patch to mesa to expose rgb565 pbuffers even on a server with
> only depth 24 and 32 visuals, fixes
> dEQP-EGL.functional.render.single_context.gles2.rgb565_pbuffer.  Those
> pbuffers (or at least something renderable with 565) are required by
> the current CTS for GLES3, and having the server support DRI3 on those
> pixmaps means that we can avoid having a different path for EGL
> pbuffers compared to pixmaps.
> 
> Signed-off-by: Eric Anholt 
> ---
>  glamor/glamor_egl.c | 22 ++
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/glamor/glamor_egl.c b/glamor/glamor_egl.c
> index b33d8ef1598e..df278b1a1a02 100644
> --- a/glamor/glamor_egl.c
> +++ b/glamor/glamor_egl.c
> @@ -280,18 +280,24 @@ glamor_make_pixmap_exportable(PixmapPtr pixmap, Bool 
> modifiers_ok)
>  (modifiers_ok || !pixmap_priv->used_modifiers))
>  return TRUE;
>  
> -if (pixmap->drawable.bitsPerPixel != 32) {
> +switch (pixmap->drawable.depth) {
> +case 30:
> +format = GBM_FORMAT_ARGB2101010;
> +break;
> +case 32:
> +case 24:
> +format = GBM_FORMAT_ARGB;
> +break;
> +case 16:
> +format = GBM_FORMAT_RGB565;
> +break;
> +default:
>  xf86DrvMsg(scrn->scrnIndex, X_ERROR,
> -   "Failed to make %dbpp pixmap exportable\n",
> -   pixmap->drawable.bitsPerPixel);
> +   "Failed to make %d depth, %dbpp pixmap exportable\n",
> +   pixmap->drawable.depth, pixmap->drawable.bitsPerPixel);
>  return FALSE;
>  }
>  
> -if (pixmap->drawable.depth == 30)
> - format = GBM_FORMAT_ARGB2101010;
> -else
> -    format = GBM_FORMAT_ARGB8888;
> -
>  #ifdef GBM_BO_WITH_MODIFIERS
>  if (modifiers_ok && glamor_egl->dmabuf_capable) {
>  uint32_t num_modifiers;
>
Reviewed-by: Michel Dänzer 


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [Pixman] [BUG] SIGSEGV in sse2_fill

2018-09-01 Thread Michel Dänzer
On 2018-09-01 9:22 a.m., Frédéric Fauberteau wrote:
> 
> [  5770.134] (EE) RADEON(0): Failed to make prime FD for handle: 22
> [  5770.134] (EE) RADEON(0): Failed to create textured screen.

This is the problem. Please try current xf86-video-ati Git master,
specifically
https://gitlab.freedesktop.org/xorg/driver/xf86-video-ati/commit/db28d35ce9fd07a2a4703f3df0633d4c8291ff9b
could help for this.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [Pixman] [BUG] SIGSEGV in sse2_fill

2018-08-30 Thread Michel Dänzer
On 2018-08-29 9:14 p.m., Adam Jackson wrote:
> continued from pixman@, original thread here:
> 
> https://lists.freedesktop.org/archives/pixman/2018-August/004759.html

Frédéric, can you attach the full corresponding Xorg log file?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [V2] modesetting: code refactor for PRIME sync

2018-08-29 Thread Michel Dänzer
On 2018-08-29 4:53 a.m., jimqu wrote:
> On 2018年08月29日 09:20, Alex Goins wrote:
>> I'm still having issues with my local setup for testing it with
>> modesetting<->modesetting PRIME, but I think it looks good by
>> inspection, and
>> would expect it to work. It does work with NVIDIA<->modesetting PRIME.
>>
>> It would be a good idea to test modesetting<->modesetting PRIME Sync
>> with this
>> patch before merging it, but I don't have any further objections.
>>
>> Reviewed-by: Alex Goins 
>>
>> Thanks!
>> Alex
> 
> Thanks very much! Alex.
> 
> I tested it on Intel(modesetting) + AMD(modesetting/amdgpu) and
> Intel(modesetting)+NV(modesetting/nouveau), PRIME, reverse PRIME and
> also PRIME offloading cases.
> For the NV card, I don't know what issue you encountered,  In my side ,
> at first, I found a GTX690, but PRIME can not work duo to there were two
> DRM device nodes from NV card, so there are totally three screens(one
> master and two GPUs) on the system. then I found a GTX500, it had only
> one device node.
> 
> Did you encounter the same issue?
> 
> BTW, could you do me a favor to help me push the patch to the master or
> other proper branch? I do not familiar with the processes that push the
> patch to Xorg.

Pushed to master, thanks guys.

To ssh://gitlab.freedesktop.org/xorg/xserver
   8a3ae555e..f79e53685  master -> master


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] randr: rrCheckPixmapBounding should only increase screen size

2018-08-24 Thread Michel Dänzer
On 2018-08-24 1:34 a.m., Alex Goins wrote:
> Hi Adam,
> 
> Can this be merged?

Pushed, thanks.

To ssh://gitlab.freedesktop.org/xorg/xserver
   1fc20b985..a90f33721  master -> master


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: how do I build xf86-video-ati

2018-08-24 Thread Michel Dänzer
On 2018-08-23 8:05 p.m., John Lumby wrote:
> 
>> Does https://patchwork.freedesktop.org/patch/245782/ help for this
>> issue?
> 
> It gave me the clue as to what I need to do : this :
> 
> ln -s ../../../xorg-build/share/aclocal m4
> 
> After that ./autogen.sh works just fine.
> 
> Your patch doesn't create the link, just an empty directory  -I
> did not try that but I guess it will work just as well provided the
> user has exported ACLOCAL as Alan indicated.creating the explicit
> link might be more robust.

A symlink doesn't solve anything outside of your system(s). I'm looking
for a general solution, hence the patch.

Anyway, thanks for providing the information needed for diagnosing and
addressing this issue.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: how do I build xf86-video-ati

2018-08-23 Thread Michel Dänzer
On 2018-08-23 5:36 p.m., John Lumby wrote:
> 
> By the way,  a very similar problem occurs on a debian sid/buster distro :
> *** Configuring xf86-video-ati *** [139/201]
> /data/xorg_180615/xorg/driver/xf86-video-ati/autogen.sh --prefix 
> /data/xorg_180615/xorg-build --disable-Werror  --without-xmlto --without-fop 
> --without-xsltproc 
> autoreconf: Entering directory `.'
> autoreconf: configure.ac: not using Gettext
> autoreconf: running: aclocal -I /data/xorg_180615/xorg-build/share/aclocal 
> --force -I m4
> aclocal: error: couldn't open directory 'm4': No such file or directory

The plot thickens. Looks like aclocal only warns if the first directory
passed via -I doesn't exist, but errors out if the second one doesn't.


Does https://patchwork.freedesktop.org/patch/245782/ help for this issue?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: how do I build xf86-video-ati

2018-08-23 Thread Michel Dänzer
On 2018-08-23 3:08 p.m., John Lumby wrote:
> 
> /usr/local/xorg-git-180820/xorg/driver/xf86-video-ati
> autoreconf: Entering directory `.'
> autoreconf: configure.ac: not using Gettext
> autoreconf: running: aclocal --force -I m4
> aclocal: warning: couldn't open directory 'm4': No such file or directory

This is just a warning.


> configure.ac:41: error: must install xorg-macros 1.8 or later before running 
> autoconf/autogen
> configure.ac:41: the top level

This looks like your problem.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: how do I build xf86-video-ati

2018-08-23 Thread Michel Dänzer
On 2018-08-23 2:53 a.m., John Lumby wrote:
> I usually build xorg using jhbuild.
> The last time I successfully built the xf86-video-ati package was in 2016,
> when the cloned build tree contained a complete autoconf setup with an 
> aclocal.m4
> and also various other build-related bits and pieces,

aclocal.m4 and other generated files have never been in the Git repository.


> But now there is no aclocal.m4 nor is there an m4 subdirectory, and  
> autogen.sh fails with
> aclocal: error: couldn't open directory 'm4': No such file or directory,

Please provide the full terminal output of running ./autogen.sh .


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] randr: rrCheckPixmapBounding should only increase screen size

2018-08-15 Thread Michel Dänzer
On 2018-08-14 10:05 PM, Alex Goins wrote:
> The purpose of rrCheckPixmapBounding() is to make sure that the fb is large
> enough to accommodate the region scanned out by a GPU screen. Currently, 
> however,
> it will actually shrink the fb if it's larger than it needs to be.
> 
> This is a problem when combining PRIME output slaving with arbitrary 
> transforms
> with xrandr.
> 
> Although arbitrary transforms are not supposed to constrain the size of the fb
> (https://lists.freedesktop.org/archives/xorg-devel/2018-January/055563.html),
> xrandr will use RRSetScreenSize to resize the desktop to accommodate scaling
> transforms, e.g. scaling a 1920x1080 display to 3840x2160 will result in a
> desktop size of 3840x2160.
> 
> In the case of PRIME, rrCheckPixmapBounding() will be called after
> RRSetScreenSize() and it will resize the fb back down to what it would be
> without the scaling transform, e.g. 1920x1080. This represents divergence in
> behavior between PRIME and non-PRIME outputs.
> 
> I had originally made rrCheckPixmapBounding() account for arbitrary 
> transforms,
> but realized that the fb being large enough to accommodate arbitrary 
> transforms
> is not a hard requirement enforced in the server. Instead, this change simply
> makes it so that rrCheckPixmapBounding() will only resize the fb to be larger
> than it already is, preventing it from stepping on prior requests to increase
> the size of the fb.
> 
> Signed-off-by: Alex Goins 
> ---
>  randr/rrcrtc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
> index 74dc5a2..cdc7bed 100644
> --- a/randr/rrcrtc.c
> +++ b/randr/rrcrtc.c
> @@ -702,8 +702,8 @@ rrCheckPixmapBounding(ScreenPtr pScreen,
>  if (new_height < screen_pixmap->drawable.height)
>  new_height = screen_pixmap->drawable.height;
>  
> -if (new_width == screen_pixmap->drawable.width &&
> -new_height == screen_pixmap->drawable.height) {
> +if (new_width <= screen_pixmap->drawable.width &&
> +    new_height <= screen_pixmap->drawable.height) {
>  } else {
>  pScrPriv->rrScreenSetSize(pScreen, new_width, new_height, 0, 0);
>  }
> 

Reviewed-by: Michel Dänzer 


P.S. Can you review https://patchwork.freedesktop.org/patch/242720/ ,
and maybe work with Jim to sort out any remaining issues? Some of the
PRIME synchronization related code in the modesetting driver is relying
on the peer screen using the modesetting driver as well.

-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] modesetting: Fix X crash in ms_dirty_update()

2018-08-03 Thread Michel Dänzer
On 2018-08-03 03:27 PM, Jim Qu wrote:
> On some Intel iGPU + AMD dGPU platform, when connect extern display
> from dGPU, X will crash, show the log like:
> 
> randr: falling back to unsynchronized pixmap sharing
> (EE)
> (EE) Backtrace:
> (EE) 0: /usr/lib/xorg/Xorg (xorg_backtrace+0x4e)
> (EE) 1: /usr/lib/xorg/Xorg (0x55cb0151a000+0x1b5ce9)
> (EE) 2: /lib/x86_64-linux-gnu/libpthread.so.0 (0x7f1587a1d000+0x11390)
> (EE)
> (EE) Segmentation fault at address 0x0
> (EE)
> 
> There is NULL pointer accessing on ent->slave_dst->drawable.pScreen->
> SharedPixmapNotifyDamage.
> 
> On the platform, since the dGPU is GPU device, so that the iGPU is
> output master device. SharedPixmapNotifyDamage() should be called when
> current device is output slave.
> 
> Change-Id: Id633e29c126670ee64ff1f5f79d489e5068cd439
> Signed-off-by: Jim Qu 
> ---
>  hw/xfree86/drivers/modesetting/driver.c | 23 ---
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/xfree86/drivers/modesetting/driver.c 
> b/hw/xfree86/drivers/modesetting/driver.c
> index 9362370..6022315 100644
> --- a/hw/xfree86/drivers/modesetting/driver.c
> +++ b/hw/xfree86/drivers/modesetting/driver.c
> @@ -640,20 +640,21 @@ ms_dirty_update(ScreenPtr screen, int *timeout)
>  xorg_list_for_each_entry(ent, >pixmap_dirty_list, ent) {
>  region = DamageRegion(ent->damage);
>  if (RegionNotEmpty(region)) {
> -msPixmapPrivPtr ppriv =
> -msGetPixmapPriv(>drmmode, ent->slave_dst);
> +if (screen->isGPU) {
> +msPixmapPrivPtr ppriv =
> +msGetPixmapPriv(>drmmode, ent->slave_dst);
>  
> -if (ppriv->notify_on_damage) {
> -ppriv->notify_on_damage = FALSE;
> +if (ppriv->notify_on_damage) {
> +ppriv->notify_on_damage = FALSE;
>  
> -ent->slave_dst->drawable.pScreen->
> -SharedPixmapNotifyDamage(ent->slave_dst);
> -}
> -
> -/* Requested manual updating */
> -if (ppriv->defer_dirty_update)
> -continue;
> +ent->slave_dst->drawable.pScreen->
> +SharedPixmapNotifyDamage(ent->slave_dst);
> +}
>  
> +/* Requested manual updating */
> +if (ppriv->defer_dirty_update)
> +continue;
> +}

I don't think this is right. E.g. why would the slave driver call its
own SharedPixmapNotifyDamage hook? Also, ppriv->defer_dirty_update is
only set to TRUE by hooks which are only called for the master screen.

So, I think the start of the new block needs to be something like:

if (!screen->isGPU) {
msPixmapPrivPtr ppriv =
msGetPixmapPriv(>drmmode,
ent->slave_dst->master_pixmap);

and msStartFlippingPixmapTracking and msStopFlippingPixmapTracking also
need to use ->master_pixmap instead of the slave pixmaps.


Not sure about the defer_dirty_update related code, that might only make
sense in the slave.


The more I look at this PRIME synchronization related code, the more I
wonder if it was ever tested with master and slave screens using
different drivers...


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: xserver: Changes to 'server-1.20-branch'

2018-08-02 Thread Michel Dänzer
On 2018-08-01 08:10 PM, GitLab Mirror wrote:
> New branch 'server-1.20-branch' available with the following commits:

I'd like to nominate https://patchwork.freedesktop.org/patch/232490/ for
1.20.1 as well.


Also, it would be nice if somebody who cares about the modesetting
driver could fix https://bugs.freedesktop.org/101998#c37 . I was about
to get fed up with it enough to do it myself, but I might not get around
to it for a couple of weeks.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] xwayland: Enable DRI3 for glamor

2018-07-20 Thread Michel Dänzer
On 2018-07-20 09:51 AM, Olivier Fourdan wrote:
> glamor_fds_from_pixmap() will bail out early if DRI3 is not enabled,
> unfortunately Xwayland's glamor code would not set it as enabled which
> would lead to blank pixmaps when using texture from pixmap.
> 
> Make sure to mark DRI3 as enabled from glamor_egl_screen_init() in
> Xwayland.
> 
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=107287

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107287

The Fixes tag is for referencing another commit which is fixed by this one.


> Signed-off-by: Olivier Fourdan 
> ---
>  hw/xwayland/xwayland-glamor.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
> index f17914344..7ea6def61 100644
> --- a/hw/xwayland/xwayland-glamor.c
> +++ b/hw/xwayland/xwayland-glamor.c
> @@ -57,6 +57,7 @@ glamor_egl_screen_init(ScreenPtr screen, struct 
> glamor_context *glamor_ctx)
>  {
>  struct xwl_screen *xwl_screen = xwl_screen_get(screen);
>  
> +glamor_enable_dri3(screen);
>  glamor_ctx->ctx = xwl_screen->egl_context;
>  glamor_ctx->display = xwl_screen->egl_display;
>  
> 

With the above fixed,

Reviewed-by: Michel Dänzer 


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xf86-video-amdgpu 3/3] Add RandR leases with modesetting driver support [v7]

2018-07-09 Thread Michel Dänzer
On 2018-07-07 02:36 AM, Keith Packard wrote:
> This adds support for RandR CRTC/Output leases through the modesetting
> driver, creating a lease using new kernel infrastructure and returning
> that to a client through an fd which will have access to only those
> resources.
> 
> [...]
> 
> + nobjects = ncrtc + noutput;
> +
> + if (nobjects == 0)
> + return BadValue;

As I mentioned on IRC, this addition can already overflow (to a non-0
value). That's one reason I decided against using xallocarray in my
patch (but instead explicitly check against overflow of the addition and
multiplication).


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xf86-video-amdgpu 0/3] Add RandR lease support [v2]

2018-07-09 Thread Michel Dänzer
On 2018-07-07 02:36 AM, Keith Packard wrote:
> When we last saw these patches, they required current X server bits to build,
> while the project requires that the driver build back to X server 1.13. I've
> added suitable checks and wrapped the new code in #ifdefs now.

Well, this is unfortunate timing. Last Friday, I finally got around to
porting these myself, as I had promised I would, but ran out of time for
sending them out before the weekend. Sent out now:

https://patchwork.freedesktop.org/series/46153/

As my patches are simpler (due to e.g. taking advantage of existing
XF86_* defines), I'm inclined to go with them. Sorry for any time you've
wasted. If you could review my patches and maybe test that the lease
functionality is actually working, that would be much appreciated.


P.S. Until we migrate to gitlab, xf86-video-amdgpu patches are normally
being reviewed on the amd-gfx mailing list.

-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: Revisiting the issue of obtaining Xorg commit privilege

2018-07-07 Thread Michel Dänzer
On 2018-07-07 04:56 AM, Kevin Brace wrote:
> Hi,
> 
> I have not seen any movement in obtaining xorg/driver commit privilege.
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=106605
> 
> Please note that several of my r128 DDX patches have been committed by Connor 
> Behan.

The r128 repository will soon be migrated to gitlab.freedesktop.org,
then it will be easier to give you write access.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCHv3] modesetting: Update fb_id from shadow allocate and destroy if not set

2018-07-06 Thread Michel Dänzer
On 2018-07-05 05:42 PM, Tony Lindgren wrote:
> Hi,
> 
> * Michel Dänzer  [180705 14:21]:
>>
>> This uses the same damage region for all framebuffers, which is
>> generally not correct for drmmode_crtc->rotate_fb_id. The coordinate
>> offset, rotation and reflection need to be taken into account for that.
> 
> Hmm OK. I thought we just need to refresh it we have
> rotate_fb_id.
> 
> Unfortunately I have no idea what the check here might be
> for the variables above.. Sounds like it might leave out
> some pointless updates though :) Care to describe a bit
> more or ideally even provide a patch to test?

The simplest solution is probably to use separate damage records
attached to the per-CRTC rotation pixmaps.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCHv3] modesetting: Update fb_id from shadow allocate and destroy if not set

2018-07-05 Thread Michel Dänzer
On 2018-07-05 11:27 AM, Tony Lindgren wrote:
> Looks like drmModeDirtyFB() stopped working at some point and we now
> call it with fb_id of zero for rotated displays. This will stop displays
> relying on DRM_IOCTL_MODE_DIRTYFB ioctl to display anything.
> 
> This regression probably with commit 3dcd591fa9b7 ("modesetting: Add
> support for using RandR shadow buffers") that inroduced rotate_fb_id.
> 
> Let's fix this issue by going through all the displays.
> 
> Cc: Hans De Goede 
> Cc: Jason Ekstrand 
> Cc: Keith Packard 
> Cc: Laurent Pinchart 
> Cc: Lyude Paul 
> Cc: Sebastian Reichel 
> Cc: Tomi Valkeinen 
> Signed-off-by: Tony Lindgren 
> ---
> 
> Here's this one resent with proper patch description, sorry
> for the delays sending it out.
> 
> ---
> 
>  hw/xfree86/drivers/modesetting/driver.c | 41 +++--
>  1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/xfree86/drivers/modesetting/driver.c 
> b/hw/xfree86/drivers/modesetting/driver.c
> --- a/hw/xfree86/drivers/modesetting/driver.c
> +++ b/hw/xfree86/drivers/modesetting/driver.c
> @@ -531,12 +531,11 @@ dispatch_dirty_region(ScrnInfoPtr scrn,
>  }
>  
>  static void
> -dispatch_dirty(ScreenPtr pScreen)
> +dispatch_dirty_fb_id(ScreenPtr pScreen, int fb_id)
>  {
>  ScrnInfoPtr scrn = xf86ScreenToScrn(pScreen);
>  modesettingPtr ms = modesettingPTR(scrn);
>  PixmapPtr pixmap = pScreen->GetScreenPixmap(pScreen);
> -int fb_id = ms->drmmode.fb_id;
>  int ret;
>  
>  ret = dispatch_dirty_region(scrn, pixmap, ms->damage, fb_id);
> @@ -547,7 +546,43 @@ dispatch_dirty(ScreenPtr pScreen)
>  ms->damage = NULL;
>  xf86DrvMsg(scrn->scrnIndex, X_INFO,
> "Disabling kernel dirty updates, not required.\n");
> -return;
> +}
> +}
> +
> +static void
> +dispatch_dirty(ScreenPtr pScreen)
> +{
> +ScrnInfoPtr scrn = xf86ScreenToScrn(pScreen);
> +modesettingPtr ms = modesettingPTR(scrn);
> +modesettingEntPtr ms_ent = ms_ent_priv(scrn);
> +xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
> +xf86CrtcPtr crtc;
> +drmmode_crtc_private_ptr drmmode_crtc;
> +unsigned int mask;
> +
> +mask = ms_ent->assigned_crtcs;
> +
> +while (mask) {
> +int i, fb_id = 0;
> +
> +i = ffs(mask) - 1;
> +
> +crtc = xf86_config->crtc[i];
> +if (!ms_crtc_on(crtc))
> +goto skip;
> +
> +drmmode_crtc = crtc->driver_private;
> +
> +if (drmmode_crtc->rotate_fb_id)
> +fb_id = drmmode_crtc->rotate_fb_id;
> +else
> +fb_id = ms->drmmode.fb_id;
> +
> +if (fb_id)
> +dispatch_dirty_fb_id(pScreen, fb_id);
> +
> +skip:
> +mask &= ~(1 << i);
>  }
>  }
>  
> 

This uses the same damage region for all framebuffers, which is
generally not correct for drmmode_crtc->rotate_fb_id. The coordinate
offset, rotation and reflection need to be taken into account for that.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: 4K@60 YCbCr420 missing mode in usermode

2018-07-02 Thread Michel Dänzer
On 2018-06-27 11:39 AM, Emil Velikov wrote:
> On 27 June 2018 at 09:40, Michel Dänzer  wrote:
>> On 2018-06-26 07:11 PM, Emil Velikov wrote:
>>> On 26 June 2018 at 17:23, Michel Dänzer  wrote:
>>>> On 2018-06-26 05:43 PM, Emil Velikov wrote:
>>>>> On 25 June 2018 at 22:45, Zuo, Jerry  wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> We are working on an issue affecting 4K@60 HDMI display not to light up, 
>>>>>> but
>>>>>> only showing up 4K@30 from:
>>>>>> https://bugs.freedesktop.org/show_bug.cgi?id=106959 and others.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Some displays (e.g., ASUS PA328) HDMI port shows YCbCr420 CEA extension
>>>>>> block with 4K@60 supported. Such HDMI 4K@60 is not real HDMI 2.0, but 
>>>>>> still
>>>>>> following HDMI 1.4 spec. with maximum TMDS clock of 300MHz instead of
>>>>>> 600MHz.
>>>>>>
>>>>>> To get such 4K@60 supported, it needs to limit the bandwidth by reducing 
>>>>>> the
>>>>>> color space to YCbCr420 only. We’ve already raised YCbCr420 only flag
>>>>>> (attached patch) from kernel side to pass the mode validation, and 
>>>>>> expose it
>>>>>> to user space.
>>>>>>
>>>>>>
>>>>>>
>>>>>> We think that one of the issues that causes this problem is due to
>>>>>> usermode pruning the 4K@60 mode from the modelist (attached Xorg.0.log). 
>>>>>> It
>>>>>> seems like when usermode receives all the modes, it doesn't take in 
>>>>>> account
>>>>>> the 4K@60 YCbCr4:2:0 specific mode. In order to pass validation of being
>>>>>> added to usermode modelist, its pixel clk needs to be divided by 2 so 
>>>>>> that
>>>>>> it won't exceed TMDS max physical pixel clk (300MHz). That might explain 
>>>>>> the
>>>>>> difference in modes between our usermode and modeset.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Such YCbCr4:2:0 4K@60 special mode is marked in DRM by raising a flag
>>>>>> (y420_vdb_modes) inside connector's display_info which can be seen in
>>>>>> do_y420vdb_modes(). Usermode could rely on that flag to pick up such mode
>>>>>> and halve the required pclk to prevent such mode getting pruned out.
>>>>>>
>>>>>>
>>>>>>
>>>>>> We were hoping for someone helps to look at it from usermode perspective.
>>>>>> Thanks a lot.
>>>>>>
>>>>> Just some observations, while going through some coffee. Take them
>>>>> with a pinch of salt.
>>>>>
>>>>> Currently the kernel edid parser (in drm core) handles the
>>>>> EXT_VIDEO_DATA_BLOCK_420 extended block.
>>>>> Additionally, the kernel allows such modes only as the (per connector)
>>>>> ycbcr_420_allowed bool is set by the driver.
>>>>>
>>>>> Quick look shows that it's only enabled by i915 on gen10 && geminilake 
>>>>> hardware.
>>>>>
>>>>> At the same time, X does it's own fairly partial edid parsing and
>>>>> doesn't handle any(?) extended blocks.
>>>>>
>>>>> One solution is to update the X parser, although it seems like a
>>>>> endless game of cat and mouse.
>>>>> IMHO a much better approach is to not use edid codepaths for KMS
>>>>> drivers (where AMDGPU is one).
>>>>> On those, the supported modes is advertised by the kernel module via
>>>>> drmModeGetConnector.
>>>>
>>>> We are getting the modes from the kernel; the issue is they are then
>>>> pruned (presumably by xf86ProbeOutputModes => xf86ValidateModesClocks)
>>>> due to violating the clock limits, as described by Jerry above.
>>>>
>>> Might have been too brief there. Here goes a more elaborate
>>> suggestion, please point out any misunderstandings.
>>>
>>> If we look into the drivers we'll see a call to xf86InterpretEDID(),
>>> followed by xf86OutputSetEDID().
>>> The former doing a partial parsing of the edid, creating a xf86MonPtr
>>> (timings information et al.) with the l

Re: 4K@60 YCbCr420 missing mode in usermode

2018-06-27 Thread Michel Dänzer
On 2018-06-26 07:11 PM, Emil Velikov wrote:
> On 26 June 2018 at 17:23, Michel Dänzer  wrote:
>> On 2018-06-26 05:43 PM, Emil Velikov wrote:
>>> On 25 June 2018 at 22:45, Zuo, Jerry  wrote:
>>>> Hello all:
>>>>
>>>>
>>>>
>>>> We are working on an issue affecting 4K@60 HDMI display not to light up, 
>>>> but
>>>> only showing up 4K@30 from:
>>>> https://bugs.freedesktop.org/show_bug.cgi?id=106959 and others.
>>>>
>>>>
>>>>
>>>> Some displays (e.g., ASUS PA328) HDMI port shows YCbCr420 CEA extension
>>>> block with 4K@60 supported. Such HDMI 4K@60 is not real HDMI 2.0, but still
>>>> following HDMI 1.4 spec. with maximum TMDS clock of 300MHz instead of
>>>> 600MHz.
>>>>
>>>> To get such 4K@60 supported, it needs to limit the bandwidth by reducing 
>>>> the
>>>> color space to YCbCr420 only. We’ve already raised YCbCr420 only flag
>>>> (attached patch) from kernel side to pass the mode validation, and expose 
>>>> it
>>>> to user space.
>>>>
>>>>
>>>>
>>>> We think that one of the issues that causes this problem is due to
>>>> usermode pruning the 4K@60 mode from the modelist (attached Xorg.0.log). It
>>>> seems like when usermode receives all the modes, it doesn't take in account
>>>> the 4K@60 YCbCr4:2:0 specific mode. In order to pass validation of being
>>>> added to usermode modelist, its pixel clk needs to be divided by 2 so that
>>>> it won't exceed TMDS max physical pixel clk (300MHz). That might explain 
>>>> the
>>>> difference in modes between our usermode and modeset.
>>>>
>>>>
>>>>
>>>> Such YCbCr4:2:0 4K@60 special mode is marked in DRM by raising a flag
>>>> (y420_vdb_modes) inside connector's display_info which can be seen in
>>>> do_y420vdb_modes(). Usermode could rely on that flag to pick up such mode
>>>> and halve the required pclk to prevent such mode getting pruned out.
>>>>
>>>>
>>>>
>>>> We were hoping for someone helps to look at it from usermode perspective.
>>>> Thanks a lot.
>>>>
>>> Just some observations, while going through some coffee. Take them
>>> with a pinch of salt.
>>>
>>> Currently the kernel edid parser (in drm core) handles the
>>> EXT_VIDEO_DATA_BLOCK_420 extended block.
>>> Additionally, the kernel allows such modes only as the (per connector)
>>> ycbcr_420_allowed bool is set by the driver.
>>>
>>> Quick look shows that it's only enabled by i915 on gen10 && geminilake 
>>> hardware.
>>>
>>> At the same time, X does it's own fairly partial edid parsing and
>>> doesn't handle any(?) extended blocks.
>>>
>>> One solution is to update the X parser, although it seems like a
>>> endless game of cat and mouse.
>>> IMHO a much better approach is to not use edid codepaths for KMS
>>> drivers (where AMDGPU is one).
>>> On those, the supported modes is advertised by the kernel module via
>>> drmModeGetConnector.
>>
>> We are getting the modes from the kernel; the issue is they are then
>> pruned (presumably by xf86ProbeOutputModes => xf86ValidateModesClocks)
>> due to violating the clock limits, as described by Jerry above.
>>
> Might have been too brief there. Here goes a more elaborate
> suggestion, please point out any misunderstandings.
> 
> If we look into the drivers we'll see a call to xf86InterpretEDID(),
> followed by xf86OutputSetEDID().
> The former doing a partial parsing of the edid, creating a xf86MonPtr
> (timings information et al.) with the latter attaching it to the
> output.
> 
> Thus as we get into xf86ProbeOutputModes/xf86ValidateModesClocks the
> Xserver probes the mode against given timing/bandwidth constrains,
> discarding it where applicable.
> 
> Considering that the DRM driver already does similar checks, X could
> side-step the parsing and filtering/validation all together.
> Trusting the kernel should be reasonable, considering weston (and I
> would imagine other wayland compositors) already do so.

It's still not clear to me what exactly you're proposing. Maybe you can
whip up at least a mock-up patch?


> Obviously, manually added modelines (via a config file) would still
> need to be validated.

How would that be done? Does the kernel provide functionality for this?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: 4K@60 YCbCr420 missing mode in usermode

2018-06-26 Thread Michel Dänzer
On 2018-06-26 05:43 PM, Emil Velikov wrote:
> Hi Jerry,
> 
> On 25 June 2018 at 22:45, Zuo, Jerry  wrote:
>> Hello all:
>>
>>
>>
>> We are working on an issue affecting 4K@60 HDMI display not to light up, but
>> only showing up 4K@30 from:
>> https://bugs.freedesktop.org/show_bug.cgi?id=106959 and others.
>>
>>
>>
>> Some displays (e.g., ASUS PA328) HDMI port shows YCbCr420 CEA extension
>> block with 4K@60 supported. Such HDMI 4K@60 is not real HDMI 2.0, but still
>> following HDMI 1.4 spec. with maximum TMDS clock of 300MHz instead of
>> 600MHz.
>>
>> To get such 4K@60 supported, it needs to limit the bandwidth by reducing the
>> color space to YCbCr420 only. We’ve already raised YCbCr420 only flag
>> (attached patch) from kernel side to pass the mode validation, and expose it
>> to user space.
>>
>>
>>
>> We think that one of the issues that causes this problem is due to
>> usermode pruning the 4K@60 mode from the modelist (attached Xorg.0.log). It
>> seems like when usermode receives all the modes, it doesn't take in account
>> the 4K@60 YCbCr4:2:0 specific mode. In order to pass validation of being
>> added to usermode modelist, its pixel clk needs to be divided by 2 so that
>> it won't exceed TMDS max physical pixel clk (300MHz). That might explain the
>> difference in modes between our usermode and modeset.
>>
>>
>>
>> Such YCbCr4:2:0 4K@60 special mode is marked in DRM by raising a flag
>> (y420_vdb_modes) inside connector's display_info which can be seen in
>> do_y420vdb_modes(). Usermode could rely on that flag to pick up such mode
>> and halve the required pclk to prevent such mode getting pruned out.
>>
>>
>>
>> We were hoping for someone helps to look at it from usermode perspective.
>> Thanks a lot.
>>
> Just some observations, while going through some coffee. Take them
> with a pinch of salt.
> 
> Currently the kernel edid parser (in drm core) handles the
> EXT_VIDEO_DATA_BLOCK_420 extended block.
> Additionally, the kernel allows such modes only as the (per connector)
> ycbcr_420_allowed bool is set by the driver.
> 
> Quick look shows that it's only enabled by i915 on gen10 && geminilake 
> hardware.
> 
> At the same time, X does it's own fairly partial edid parsing and
> doesn't handle any(?) extended blocks.
> 
> One solution is to update the X parser, although it seems like a
> endless game of cat and mouse.
> IMHO a much better approach is to not use edid codepaths for KMS
> drivers (where AMDGPU is one).
> On those, the supported modes is advertised by the kernel module via
> drmModeGetConnector.

We are getting the modes from the kernel; the issue is they are then
pruned (presumably by xf86ProbeOutputModes => xf86ValidateModesClocks)
due to violating the clock limits, as described by Jerry above.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: gitlab migration

2018-06-12 Thread Michel Dänzer
On 2018-06-12 01:35 PM, Daniel Stone wrote:
> On 11 June 2018 at 11:33, Michel Dänzer  wrote:
>> On 2018-06-08 08:08 PM, Adam Jackson wrote:
>>> I'd like us to start moving repos and bug tracking into gitlab.
>>> Hopefully everyone's aware that gitlab exists and why fdo projects are
>>> migrating to it. If not, the thread about Mesa's migration provides
>>> some useful background:
>>>
>>> https://lists.x.org/archives/mesa-dev/2018-May/195634.html
>>>
>>> This should be mostly mechanical, except for moving some of the older
>>> junk into the archive and deciding which drivers _not_ to move yet (I
>>> imagine intel has some of its processes hooked up to bz, for example).
>>
>> As the maintainer of xf86-video-amdgpu and -ati, I'm fine with migrating
>> these to GitLab for Git and patch review.
>>
>> However, I'm not sure what to do about bugs/issues. My first thought was
>> to allow creating new issues in GitLab and disable creating new reports
>> in Bugzilla, but not to migrate existing reports from Bugzilla. However,
>> it still happens fairly often that a report is initially filed against
>> the Xorg drivers, even though it's actually an issue in the X server,
>> Mesa or the kernel (old habits die hard...). Therefore, I'm inclined to
>> stick to Bugzilla until at least the X server and Mesa have moved to
>> GitLab for issue tracking, to hopefully allow moving such misfiled issues.
> 
> One thing some Mesa people said during that discussion is that they
> like the ability to move issues between Mesa and the kernel, which
> won't be possible if they're on split systems. So I probably wouldn't
> hold my breath for that to be honest.

Then I'd rather not use GitLab issues for these drivers at all either
for the time being.

The same argument applies to xserver as well, but I won't stand in the
way of migrating that to GitLab issues.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver v3] xwayland: use pixmap size on present flip

2018-06-11 Thread Michel Dänzer
On 2018-06-10 06:34 PM, Roman Gilg wrote:
> The only question open to me is how the window and pixmap size can be
> different at all at this point. But taking the Pixmap size is in any
> case better.

Agreed on both counts.


> I reviewed v3 on patchworks.
> 
> Reviewed-by: Roman Gilg 

Pushed with this and a Fixes: tag added, thanks Olivier and Roman!


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: gitlab migration

2018-06-11 Thread Michel Dänzer
On 2018-06-08 08:08 PM, Adam Jackson wrote:
> I'd like us to start moving repos and bug tracking into gitlab.
> Hopefully everyone's aware that gitlab exists and why fdo projects are
> migrating to it. If not, the thread about Mesa's migration provides
> some useful background:
> 
> https://lists.x.org/archives/mesa-dev/2018-May/195634.html
> 
> This should be mostly mechanical, except for moving some of the older
> junk into the archive and deciding which drivers _not_ to move yet (I
> imagine intel has some of its processes hooked up to bz, for example).

As the maintainer of xf86-video-amdgpu and -ati, I'm fine with migrating
these to GitLab for Git and patch review.

However, I'm not sure what to do about bugs/issues. My first thought was
to allow creating new issues in GitLab and disable creating new reports
in Bugzilla, but not to migrate existing reports from Bugzilla. However,
it still happens fairly often that a report is initially filed against
the Xorg drivers, even though it's actually an issue in the X server,
Mesa or the kernel (old habits die hard...). Therefore, I'm inclined to
stick to Bugzilla until at least the X server and Mesa have moved to
GitLab for issue tracking, to hopefully allow moving such misfiled issues.


Adding the amd-gfx list, in cases somebody there has concerns or other
feedback.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 1/2] xwayland: simplify xwl_glamor_pixmap_get_wl_buffer()

2018-06-11 Thread Michel Dänzer
On 2018-06-11 10:22 AM, Olivier Fourdan wrote:
> When retrieving the Wayland buffer from a pixmap, if the buffer already
> exists, the GBM backend will return that existing buffer.
> 
> However, as seen with the Present issues, if the call had previously
> passed a wrong size, that buffer will remain at the wrong size for as
> long as the buffer exists, which is error prone.
> 
> Considering that the width/height passed to get_wl_buffer() is always the
> actual pixmap  drawable size, and considering that the EGLStream backend
> makes no use of the size either, there is really no point in passing the
> width/height around.
> 
> Simplify the xwl_glamor_pixmap_get_wl_buffer() and EGL backends API by
> removing the pixmap size, and use the drawable size instead.
> 
> Signed-off-by: Olivier Fourdan 

Reviewed-by: Michel Dänzer 

Since the Wayland buffer represents the pixmap, their dimensions must
always match.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] xwayland: use pixmap size on present flip

2018-06-08 Thread Michel Dänzer
On 2018-06-08 04:13 PM, Olivier Fourdan wrote:
> If the pixmap size does not match the present box size, flickering
> occurs.
> 
> This can happen when the client changes its size (e.g. switching to
> fullscreen), and since the buffer is kept as long as the pixmap is
> valid, once the buffer is created, it remains at the wrong (old) size
> and causes continuous flickering.
> 
> Use the actual pixmap's drawable size instead of the present box to
> create the buffer so that it's sized appropriately.
> 
> Bugzilla: https://bugs.freedesktop.org/106841
> Signed-off-by: Olivier Fourdan 
> ---
>  hw/xwayland/xwayland-present.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
> index 4db0d1efc..32727310f 100644
> --- a/hw/xwayland/xwayland-present.c
> +++ b/hw/xwayland/xwayland-present.c
> @@ -458,8 +458,8 @@ xwl_present_flip(WindowPtr present_window,
>  xwl_window->present_window = present_window;
>  
>  buffer = xwl_glamor_pixmap_get_wl_buffer(pixmap,
> - present_box->x2 - 
> present_box->x1,
> - present_box->y2 - 
> present_box->y1,
> + pixmap->drawable.width,
> + pixmap->drawable.height,
>   _created);
>  
>  event->event_id = event_id;
> 

Please remove the present_box local, it's unused now. With that,

Reviewed-by: Michel Dänzer 


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] xwayland: fix typo in non-modifier fallback path

2018-06-08 Thread Michel Dänzer
On 2018-05-29 02:39 AM, Dave Airlie wrote:
> From: Dave Airlie 
> 
> Pointed out on irc by q66.
> ---
>  hw/xwayland/xwayland-glamor-gbm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/xwayland/xwayland-glamor-gbm.c 
> b/hw/xwayland/xwayland-glamor-gbm.c
> index 29325adac..68c2cc32e 100644
> --- a/hw/xwayland/xwayland-glamor-gbm.c
> +++ b/hw/xwayland/xwayland-glamor-gbm.c
> @@ -272,7 +272,7 @@ xwl_glamor_gbm_get_wl_buffer_for_pixmap(PixmapPtr pixmap,
>  #else
>  num_planes = 1;
>  modifier = DRM_FORMAT_MOD_INVALID;
> -strides[0] = gbm_go_get_stride(xwl_pixmap->bo);
> +strides[0] = gbm_bo_get_stride(xwl_pixmap->bo);
>  offsets[0] = 0;
>  #endif
>  
> 

Pushed, thanks!


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver] present/wnmd: Preserve window pixmap's screen_x/y on flip

2018-06-07 Thread Michel Dänzer
From: Michel Dänzer 

The incorrect values could result in the new pixmap's contents
getting corrupted down the line.

Bugzilla: https://bugs.freedesktop.org/106841
Fixes: 029608dd8020 "present: Add window flip mode"
Signed-off-by: Michel Dänzer 
---
 present/present_wnmd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/present/present_wnmd.c b/present/present_wnmd.c
index 80ffb014e..1e3958bfc 100644
--- a/present/present_wnmd.c
+++ b/present/present_wnmd.c
@@ -469,6 +469,8 @@ present_wnmd_execute(present_vblank_ptr vblank, uint64_t 
ust, uint64_t crtc_msc)
 PixmapPtr old_pixmap = screen->GetWindowPixmap(window);
 
 /* Replace window pixmap with flip pixmap */
+vblank->pixmap->screen_x = old_pixmap->screen_x;
+vblank->pixmap->screen_y = old_pixmap->screen_y;
 present_set_tree_pixmap(toplvl_window, old_pixmap, 
vblank->pixmap);
 vblank->pixmap->refcnt++;
 dixDestroyPixmap(old_pixmap, old_pixmap->drawable.id);
-- 
2.17.1

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] exa: Use PictureMatchFormat for source-only picture format description

2018-06-07 Thread Michel Dänzer
On 2018-06-06 10:26 PM, Adam Jackson wrote:
> On Fri, 2018-06-01 at 11:58 +0200, Michel Dänzer wrote:
>> From: Michel Dänzer 
>>
>> Their pFormat member is NULL, which resulted in a crash in
>> miRenderColorToPixel.
>>
>> Fixes: 8171d4c2d67b "render: Store and use all 16bpc of precision for
>>      solid pixels (v2.1)"
>> Signed-off-by: Michel Dänzer 
> 
> I'm not entirely sure why a source-only picture would have ->format but
> not ->pFormat?

static PicturePtr
createSourcePicture(void)
{
[...]
pPicture->pFormat = 0;
[...]
pPicture->format = PICT_a8r8g8b8;
[...]
}

The immediate issue is that getting the PictFormatPtr requires the
ScreenPtr, which isn't readily available here. Probably solvable, but
not trivial, so I consider it a SEP. :)


> Still, if that's the case, this would definitely be a correct fix, so.
> 
> Reviewed-by: Adam Jackson 

Thanks!


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: Obtaining Xorg DDX commit privilege

2018-06-05 Thread Michel Dänzer
On 2018-06-05 05:00 AM, Kevin Brace wrote:
> 
> I do have OpenChrome related commit privilege and I wanted to extend it to 
> other xf86-video-* DDXs.
> I did post several patches over at xorg-driver-ati mailing list if that is 
> what is needed to earn this privilege.

Connor Behan has been looking after xf86-video-r128.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver] exa: Use PictureMatchFormat for source-only picture format description

2018-06-01 Thread Michel Dänzer
From: Michel Dänzer 

Their pFormat member is NULL, which resulted in a crash in
miRenderColorToPixel.

Fixes: 8171d4c2d67b "render: Store and use all 16bpc of precision for
 solid pixels (v2.1)"
Signed-off-by: Michel Dänzer 
---
 exa/exa_render.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/exa/exa_render.c b/exa/exa_render.c
index 50a9a659e..9fbfdfca2 100644
--- a/exa/exa_render.c
+++ b/exa/exa_render.c
@@ -291,7 +291,8 @@ exaTryDriverSolidFill(PicturePtr pSrc,
 pixel = exaGetPixmapFirstPixel(pSrcPix);
 }
 else
-miRenderColorToPixel(pSrc->pFormat,
+miRenderColorToPixel(PictureMatchFormat(pDst->pDrawable->pScreen, 32,
+pSrc->format),
  >pSourcePict->solidFill.fullcolor,
  );
 
-- 
2.17.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 2/2] glamor: Propagate glamor_fds_from_pixmap error in glamor_fd_from_pixmap

2018-05-28 Thread Michel Dänzer
On 2018-05-23 05:57 PM, Emil Velikov wrote:
> On 23 May 2018 at 10:43, Michel Dänzer <mic...@daenzer.net> wrote:
>> From: Michel Dänzer <michel.daen...@amd.com>
>>
>> glamor_fds_from_pixmap returns 0 on error, but we were treating that as
>> success, continuing with uninitialized stride and fd values.
>>
>> Also bail if the offset isn't 0, same as in dri3_fd_from_pixmap.
>>
>> Fixes: c8c276c9569b "glamor: Implement PixmapFromBuffers and
>>  BuffersFromPixmap"
>> Signed-off-by: Michel Dänzer <michel.daen...@amd.com>
> 
> I would suggest a one-liner as below. Smaller patch, plus is reads
> easier on my end ;-)
> 
> -if (ret > 1) {
> +if (ret != 1 || offsets[0] != 0) {

Thanks for the suggestion.


> Regardless, the series is on point and is
> Reviewed-by: Emil Velikov <emil.veli...@collabora.com>

Pushed both fixes (with the above incorporated), thanks!


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] glamor: Work around GEM usage

2018-05-24 Thread Michel Dänzer
On 2018-05-23 10:58 PM, Thomas Hellstrom wrote:
> On 05/23/2018 08:00 PM, Adam Jackson wrote:
>> On Wed, 2018-05-23 at 11:14 +0200, Thomas Hellstrom wrote:
>>> KMS drivers are not required to support GEM. In particular, vmwgfx
>>> doesn't support flink and handles and names are identical.
>>> Getting a bo name should really be part of a lower level API, if needed,
>>> but in the mean time work around this by setting the name identical to
>>> the handle if GEM isn't supported.
>> I'd thought flink was old and busted anyway. Could this path just go
>> away?
> 
> I guess it's needed for dri2? If all drivers using glamor are OK with
> dropping dri2, then that should be OK.

FWIW, our drivers don't use glamor_name_from_pixmap, so they can support
DRI2 regardless.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 2/2] glamor: Propagate glamor_fds_from_pixmap error in glamor_fd_from_pixmap

2018-05-23 Thread Michel Dänzer
From: Michel Dänzer <michel.daen...@amd.com>

glamor_fds_from_pixmap returns 0 on error, but we were treating that as
success, continuing with uninitialized stride and fd values.

Also bail if the offset isn't 0, same as in dri3_fd_from_pixmap.

Fixes: c8c276c9569b "glamor: Implement PixmapFromBuffers and
 BuffersFromPixmap"
Signed-off-by: Michel Dänzer <michel.daen...@amd.com>
---
 glamor/glamor.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/glamor/glamor.c b/glamor/glamor.c
index e2c74d17a..63f0947fa 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -865,17 +865,15 @@ glamor_fd_from_pixmap(ScreenPtr screen,
  );
 
 /* Pixmaps with multi-planes/modifier are not supported in this interface 
*/
-if (ret > 1) {
-while (ret > 0)
-close(fds[--ret]);
-return -1;
+if (ret == 1 && offsets[0] == 0) {
+*stride = strides[0];
+*size = pixmap->drawable.height * *stride;
+return fds[0];
 }
 
-ret = fds[0];
-*stride = strides[0];
-*size = pixmap->drawable.height * *stride;
-
-return ret;
+while (ret > 0)
+close(fds[--ret]);
+return -1;
 }
 
 _X_EXPORT int
-- 
2.17.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 1/2] glamor: Always return 0 from glamor_fds_from_pixmap on error

2018-05-23 Thread Michel Dänzer
From: Michel Dänzer <michel.daen...@amd.com>

This matches what glamor_egl_fds_from_pixmap and dri3_fds_from_pixmap do
and what proc_dri3_buffers_from_pixmap expects.

Fixes: c8c276c9569b "glamor: Implement PixmapFromBuffers and
 BuffersFromPixmap"
Signed-off-by: Michel Dänzer <michel.daen...@amd.com>
---
 glamor/glamor.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/glamor/glamor.c b/glamor/glamor.c
index d984d20f3..e2c74d17a 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -836,20 +836,20 @@ glamor_fds_from_pixmap(ScreenPtr screen, PixmapPtr 
pixmap, int *fds,
 glamor_get_screen_private(pixmap->drawable.pScreen);
 
 if (!glamor_priv->dri3_enabled)
-return -1;
+return 0;
 switch (pixmap_priv->type) {
 case GLAMOR_TEXTURE_DRM:
 case GLAMOR_TEXTURE_ONLY:
 if (!glamor_pixmap_ensure_fbo(pixmap, pixmap->drawable.depth == 30 ?
   GL_RGB10_A2 : GL_RGBA, 0))
-return -1;
+return 0;
 return glamor_egl_fds_from_pixmap(screen, pixmap, fds,
   strides, offsets,
   modifier);
 default:
 break;
 }
-return -1;
+return 0;
 }
 
 _X_EXPORT int
-- 
2.17.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 2/2] modesetting: Pass O_CLOEXEC when opening a DRM device

2018-05-18 Thread Michel Dänzer
From: Michel Dänzer <michel.daen...@amd.com>

We don't want DRM file descriptors to leak to child processes.

Signed-off-by: Michel Dänzer <michel.daen...@amd.com>
---
 hw/xfree86/drivers/modesetting/driver.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/xfree86/drivers/modesetting/driver.c 
b/hw/xfree86/drivers/modesetting/driver.c
index 5d8906d63..306541f33 100644
--- a/hw/xfree86/drivers/modesetting/driver.c
+++ b/hw/xfree86/drivers/modesetting/driver.c
@@ -200,12 +200,12 @@ open_hw(const char *dev)
 int fd;
 
 if (dev)
-fd = open(dev, O_RDWR, 0);
+fd = open(dev, O_RDWR | O_CLOEXEC, 0);
 else {
 dev = getenv("KMSDEVICE");
-if ((NULL == dev) || ((fd = open(dev, O_RDWR, 0)) == -1)) {
+if ((NULL == dev) || ((fd = open(dev, O_RDWR | O_CLOEXEC, 0)) == -1)) {
 dev = "/dev/dri/card0";
-fd = open(dev, O_RDWR, 0);
+fd = open(dev, O_RDWR | O_CLOEXEC, 0);
 }
 }
 if (fd == -1)
-- 
2.17.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 1/2] xfree86: Fix O_CLOEXEC usage in lnx_platform

2018-05-18 Thread Michel Dänzer
From: Michel Dänzer <michel.daen...@amd.com>

It was passing O_CLOEXEC as permission bits instead of as a flag.

Signed-off-by: Michel Dänzer <michel.daen...@amd.com>
---
 hw/xfree86/os-support/linux/lnx_platform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/xfree86/os-support/linux/lnx_platform.c 
b/hw/xfree86/os-support/linux/lnx_platform.c
index 11af52c46..70374ace8 100644
--- a/hw/xfree86/os-support/linux/lnx_platform.c
+++ b/hw/xfree86/os-support/linux/lnx_platform.c
@@ -43,7 +43,7 @@ get_drm_info(struct OdevAttributes *attribs, char *path, int 
delayed_index)
 }
 
 if (fd == -1)
-fd = open(path, O_RDWR, O_CLOEXEC);
+fd = open(path, O_RDWR | O_CLOEXEC, 0);
 
 if (fd == -1)
 return FALSE;
-- 
2.17.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: Stepping back

2018-05-15 Thread Michel Dänzer
On 2018-05-15 07:43 AM, Alan Coopersmith wrote:
> On 05/14/18 12:58 PM, Keith Packard wrote:
>> Adam Jackson <a...@nwnk.net> writes:
>>
>>> As for what this means for tree management and future release plans,
>>> well, I can't answer that, that's sort of the point. There's a
>>> community discussion that needs to happen there, and my opinions can't 
>>> dominate that if I'm serious about stepping back.
>>
>> We can start discussions now, and I think we should plan on holding a
>> discussion about this during XDC in September.
> 
> While in person discussions can be efficient, I do wonder if limiting
> them to people who can travel to XDC is how we end up burning out the
> same folks over and over.
> 
> I don't have great answers as more of us step back like I did a few
> years back and Adam is doing now, but with few new folks coming into
> our maintainer fold in the past decade, we need to figure out how to
> both get more contributors and how to grow them into maintainers.
> 
> While Wayland is certainly making great strides, I don't think the world
> is yet ready to stop using X altogether, but it's getting harder and
> harder for us to keep X alive.

FWIW, since all major Linux distros are now using Wayland by default, I
do think it's a good time to transition the Xorg DDX to more of a
maintenance mode. Any significant new functionality should be developed
for Wayland first, adding it to Xorg as well should only be considered
second.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: Stepping back

2018-05-15 Thread Michel Dänzer
On 2018-05-14 09:50 PM, Adam Jackson wrote:
> tl;dr: I will not be release manager for 1.21, nor for anything
> thereafter either, and this time that's probably permanent.
> 
> I won't make this too long to read though, it's not complicated. In a
> technical sense, I don't feel that I've been an effective release
> manager, evidenced if by nothing else than the wild gap between
> projected and actual release dates for 1.20. I don't feel capable of
> being all of individual contributor, reviewer of last resort, arbiter
> of taste and technical direction, and tree sheriff; I certainly don't
> feel capable of fulfilling all those roles while working largely in
> isolation.

I think you're being too hard on yourself here; you've done a tremendous
job, thank you!


> From the project's perspective, I don't think it's healthy for the same
> roles to continue to be held by the same few people.

That's certainly true.


> And, personally, I've been more or less focused on xserver for well
> over a decade, and I badly need a change of scenery.

I can relate to that feeling. :)


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [Mesa-dev] [PATCH 2/2] loader_dri3: Variant 2: Wait for pending swaps to complete before drawable_fini.

2018-05-11 Thread Michel Dänzer
On 2018-05-09 07:32 PM, Adam Jackson wrote:
> On Tue, 2018-05-08 at 11:42 +0200, Michel Dänzer wrote:
> 
>> Idle notify events shouldn't need special treatment, since the pixmap
>> XIDs of the buffers will be different between loader_dri3_drawable
>> incarnations, aren't they?
> 
> We're destroying loader_dri3_drawable structs at MakeCurrent time, but
> not destroying actual drawables, so I don't think the XIDs will change.

I'm talking about loader_dri3_buffer::pixmap, which are destroyed in
loader_dri3_drawable_fini -> dri3_free_render_buffer.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: Re: [RFC] Fix attempt for Mesa + X-Server 1.20 + modesetting-ddx hangs on KDE5.

2018-05-08 Thread Michel Dänzer
On 2018-05-08 06:41 PM, Adam Jackson wrote:
> On Fri, 2018-05-04 at 15:45 +0200, Mario Kleiner wrote:
> 
>> The real problem, if i understand it correctly, is the way the life-time
>> of dri3_drawables and loader_dri3_drawables is managed atm. by Mesa's
>> bindContext() functions. Whenever glXMakeCurrent() etc. are called to
>> assign new/different GLXDrawables to the same context (ie. one context
>> reused for drawing into many different drawables, as opposed to using
>> one dedicated context for each drawable), we destroy the underlying
>> DRIDrawables/dri3_drawables_loader_dri3_drawables and they lose all
>> state wrt. pending bufferswaps, msc, sbc, ust.
> 
> That's utterly, utterly, utterly broken.
> 
>> Therefore one of these patches is either a good enough fix for the KDE
>> hang problems atm. or a diagnosis of the problem as a starting point for
>> brighter people to deal with the root cause ;-)
> 
> I'll see what I can come up with. I'm not sure there's a great fix for
> this that doesn't involve a few more roundtrips at MakeCurrent time,
> since we can lose drawables asynchronously, but such is life.

I had an idea, at least for SBC:

In dri3_destroy_drawable, store the drawable's send_sbc value in a hash
table (keyed on the XID) in struct dri3_screen. Then in
dri3_create_drawable, if there's an entry for the drawable's XID in the
hash table, initialize send_sbc and recv_sbc to that.

If nobody beats me to it, I'll try this tomorrow.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [Mesa-dev] [PATCH 2/2] loader_dri3: Variant 2: Wait for pending swaps to complete before drawable_fini.

2018-05-08 Thread Michel Dänzer
On 2018-05-05 06:25 AM, Mario Kleiner wrote:
> On Sat, May 5, 2018 at 4:08 AM, Mike Lothian <m...@fireburn.co.uk> wrote:
>> I definately saw the steam bug with patch 1 but not with plasmashell,
>> I started seeing it with patch 2 but it seemed to fix itself
> 
> I had two hangs of kwin_x11 within the last 6 hours when alt-tabbing
> between windows, where it got stuck in the
> loader_dri3_swapbuffer_barrier() from patch 1/2. Not sure how that is
> possible, or if the stacktrace was misleading, because i had to VT
> switch to a text console to attach the debugger and this might be just
> a side effect of that. But if it is true, then patch 1/2 would not be
> it. Also 1/2 has a potential performance impact, whereas 2/2 doesn't.
> However 2/2 would also need more work, as i can think of more complex
> scenarios where it would filter the wrong events, although not in the
> case of plasmashell or steam. Probably we'd need to sacrifice a few
> sbc bits in the Present events serial field to transport a unique tag
> for each incarnation of the loader_dri3_drawable, like a mini-hash of
> the draw->eid. Ugly ugly...

How about the below?

Idle notify events shouldn't need special treatment, since the pixmap
XIDs of the buffers will be different between loader_dri3_drawable
incarnations, aren't they?


This still leaves the issue that the SBC moves backwards, which could
theoretically result in hangs with apps using glXWaitForSbcOML. Fixing
that would probably require changing the loader_dri3_drawable lifetime
cycle, which would probably be very invasive, if feasible at all. Maybe
we don't need to care about that for the time being, until there's a
real world app running into it.


diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
index 6db8303d26d..f0ff2f07bde 100644
--- a/src/loader/loader_dri3_helper.c
+++ b/src/loader/loader_dri3_helper.c
@@ -370,9 +370,17 @@ dri3_handle_present_event(struct loader_dri3_drawable 
*draw,
* checking for wrap.
*/
   if (ce->kind == XCB_PRESENT_COMPLETE_KIND_PIXMAP) {
- draw->recv_sbc = (draw->send_sbc & 0xLL) | ce->serial;
- if (draw->recv_sbc > draw->send_sbc)
-draw->recv_sbc -= 0x1;
+ uint64_t recv_sbc = (draw->send_sbc & 0xLL) | 
ce->serial;
+
+ /* Only assume wraparound if that results in exactly the previous
+  * SBC + 1, otherwise ignore received SBC > sent SBC (those are
+  * probably from a previous loader_dri3_drawable instance) to avoid
+  * calculating bogus target MSC values in loader_dri3_swap_buffers_msc
+  */
+ if (recv_sbc <= draw->send_sbc)
+draw->recv_sbc = recv_sbc;
+ else if (recv_sbc == (draw->recv_sbc + 0x10001ULL))
+draw->recv_sbc = recv_sbc - 0x1ULL;

  /* When moving from flip to copy, we assume that we can allocate in
   * a more optimal way if we don't need to cater for the display


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [ANNOUNCE] xorg-server 1.19.99.905

2018-04-25 Thread Michel Dänzer
On 2018-04-24 11:17 PM, Adam Jackson wrote:
> More bugfixes, and streams support for Xwayland. This will almost
> certainly be the last RC.

What about the Xwayland Present issue with unmapped windows (affecting
e.g. skypeforlinux)?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer



signature.asc
Description: OpenPGP digital signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] present: fix msc offset calculation in window mode

2018-04-20 Thread Michel Dänzer
On 2018-04-20 05:10 PM, Roman Gilg wrote:
> Instead of getting the current msc value from the window, which might be
> different to old one directly take the last saved msc value saved in
> the window_priv struct.
> 
> Signed-off-by: Roman Gilg <subd...@gmail.com>
> ---
>  present/present_wnmd.c | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/present/present_wnmd.c b/present/present_wnmd.c
> index 72bda2f..80ffb01 100644
> --- a/present/present_wnmd.c
> +++ b/present/present_wnmd.c
> @@ -518,8 +518,6 @@ present_wnmd_window_to_crtc_msc(WindowPtr window, 
> RRCrtcPtr crtc, uint64_t windo
>  present_window_priv_ptr window_priv = present_get_window_priv(window, 
> TRUE);
>  
>  if (crtc != window_priv->crtc) {
> -uint64_t old_ust, old_msc;
> -
>  if (window_priv->crtc == PresentCrtcNeverSet) {
>  window_priv->msc_offset = 0;
>  } else {
> @@ -527,10 +525,7 @@ present_wnmd_window_to_crtc_msc(WindowPtr window, 
> RRCrtcPtr crtc, uint64_t windo
>   * we'll just use whatever previous MSC we'd seen from this CRTC
>   */
>  
> -if (present_wnmd_get_ust_msc(window->drawable.pScreen, window, 
> _ust, _msc) != Success)
> -old_msc = window_priv->msc;
> -
> -window_priv->msc_offset += new_msc - old_msc;
> +window_priv->msc_offset += new_msc - window_priv->msc;

This is working around an issue in xwl_present_get_ust_msc (it doesn't
return consistent MSC values for a window which has been unmapped and
mapped again). This change might do the wrong thing with another backend
which has real CRTCs and returns consistent MSC values for them.

That said, maybe this is the best that can be done to address the
immediate issue, but it might be good to at least add a comment saying
this is a kludge which should be revisited after 1.20.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver v3 3/3] modesetting: Actually get framebuffer ID

2018-04-05 Thread Michel Dänzer
On 2018-04-05 05:47 PM, Daniel Stone wrote:
> We would fail to get the FB ID if it wasn't already imported, since we
> were checking to see if the pointer was NULL (it never was) rather than
> if the content of the pointer was 0.
> 
> Signed-off-by: Daniel Stone <dani...@collabora.com>
> Reported-by: Olivier Fourdan <ofour...@redhat.com>
> ---
>  hw/xfree86/drivers/modesetting/drmmode_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
> b/hw/xfree86/drivers/modesetting/drmmode_display.c
> index cdcd4f3f3..322ef050b 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> @@ -624,7 +624,7 @@ drmmode_crtc_get_fb_id(xf86CrtcPtr crtc, uint32_t *fb_id, 
> int *x, int *y)
>  *y = crtc->y;
>  }
>  
> -if (fb_id == 0) {
> +if (*fb_id == 0) {
>  ret = drmmode_bo_import(drmmode, >front_bo,
>          >fb_id);
>  if (ret < 0) {
> 

Reviewed-and-Tested-by: Michel Dänzer <michel.daen...@amd.com>


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 1/2] modesetting: Don't reuse iterator in nested loop

2018-04-05 Thread Michel Dänzer
On 2018-04-05 03:58 PM, Daniel Stone wrote:
> drmmode_crtc_set_mode has a loop nested inside another loop, where both
> of them were using 'i' as the loop iterator. Rename it to avoid an
> infinite loop.
> 
> Signed-off-by: Daniel Stone <dani...@collabora.com>
> Reported-by: Michel Dänzer <michel.daen...@amd.com>
> ---
>  hw/xfree86/drivers/modesetting/drmmode_display.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
> b/hw/xfree86/drivers/modesetting/drmmode_display.c
> index 0eacefba2..f5f9fa582 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> @@ -778,12 +778,13 @@ drmmode_crtc_set_mode(xf86CrtcPtr crtc, Bool test_only)
>  drmmode_crtc_private_ptr other_drmmode_crtc = 
> other_crtc->driver_private;
>  int lost_outputs = 0;
>  int remaining_outputs = 0;
> + int j;

Indentation looks wrong here.


With that and the bug pointed out by others fixed,

Reviewed-and-Tested-by: Michel Dänzer <michel.daen...@amd.com>


Note that it still fails to actually light up for me with amdgpu DC
though, drmModeAtomicCommit returns -EINVAL. I'm afraid no bandwidth
right now (or indeed for a while) to help with looking into that, other
than maybe occasionally testing patches. Adding the amd-gfx list, as
this might be a DC issue.

OTOH it fails to light up with the radeon kernel driver as well,
drmModeSetCrtc returns -ENOENT.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] build: Bump Mesa requirement to 17.1

2018-03-15 Thread Michel Dänzer
On 2018-03-14 07:43 PM, Adam Jackson wrote:
> gbm_bo_get_modifier is new in 17.1, which is 10 months old and two
> stable branches ago.
> 
> Signed-off-by: Adam Jackson <a...@redhat.com>
> ---
>  configure.ac| 2 +-
>  glx/meson.build | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index f82c0a66a..18cfda69e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1113,7 +1113,7 @@ case "$DRI2,$HAVE_DRI2PROTO" in
>   yes,yes | auto,yes)
>   AC_DEFINE(DRI2, 1, [Build DRI2 extension])
>   DRI2=yes
> - LIBGL="gl >= 9.2.0"
> + LIBGL="gl >= 17.1"
>   SDK_REQUIRED_MODULES="$SDK_REQUIRED_MODULES $DRI2PROTO"
>   ;;
>  esac
> diff --git a/glx/meson.build b/glx/meson.build
> index 5f93a75a5..3b0618f67 100644
> --- a/glx/meson.build
> +++ b/glx/meson.build
> @@ -39,7 +39,7 @@ if build_glx
>  common_dep,
>  dl_dep,
>  dependency('glproto', version: '>= 1.4.17'),
> -dependency('gl', version: '>= 9.2.0'),
> +dependency('gl', version: '>= 17.1.0'),
>  ],
>  c_args: [
>  glx_align64,
> @@ -70,7 +70,7 @@ if build_glx
>  common_dep,
>  dl_dep,
>  dependency('glproto', version: '>= 1.4.17'),
> -dependency('gl', version: '>= 9.2.0'),
> +dependency('gl', version: '>= 17.1.0'),
>  ],
>  )
>  endif
> 

Reviewed-by: Michel Dänzer <michel.daen...@amd.com>


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver] randr: Initialize RROuptutRec::nonDesktop

2018-03-14 Thread Michel Dänzer
From: Michel Dänzer <michel.daen...@amd.com>

Flagged by valgrind:

==13695== Conditional jump or move depends on uninitialised value(s)
==13695==at 0x22461C: RRNoticePropertyChange (rrproperty.c:150)
==13695==by 0x22461C: RRChangeOutputProperty (rrproperty.c:263)
==13695==by 0x222FC4: RROutputSetNonDesktop (rroutput.c:333)
==13695==by 0x22319C: RROutputCreate (rroutput.c:122)
==13695==by 0x1E1CE9: xf86RandR12CreateObjects12 (xf86RandR12.c:1734)
==13695==by 0x1E1CE9: xf86RandR12Init12 (xf86RandR12.c:2375)
==13695==by 0x1E1CE9: xf86RandR12Init (xf86RandR12.c:895)
==13695==by 0x1D469B: xf86CrtcScreenInit (xf86Crtc.c:778)
==13695==by 0xC095A54: RADEONScreenInit_KMS (radeon_kms.c:2436)
==13695==by 0x161444: AddGPUScreen (dispatch.c:3966)
==13695==by 0x1A3E46: InitOutput (xf86Init.c:763)
==13695==by 0x1654A7: dix_main (main.c:193)
==13695==by 0x7041A86: (below main) (libc-start.c:310)
==13695==  Uninitialised value was created by a heap allocation
==13695==at 0x4C2CB8F: malloc (vg_replace_malloc.c:299)
==13695==by 0x223083: RROutputCreate (rroutput.c:83)
==13695==by 0x1E1CE9: xf86RandR12CreateObjects12 (xf86RandR12.c:1734)
==13695==by 0x1E1CE9: xf86RandR12Init12 (xf86RandR12.c:2375)
==13695==by 0x1E1CE9: xf86RandR12Init (xf86RandR12.c:895)
==13695==by 0x1D469B: xf86CrtcScreenInit (xf86Crtc.c:778)
==13695==by 0xC095A54: RADEONScreenInit_KMS (radeon_kms.c:2436)
==13695==by 0x161444: AddGPUScreen (dispatch.c:3966)
==13695==by 0x1A3E46: InitOutput (xf86Init.c:763)
==13695==by 0x1654A7: dix_main (main.c:193)
==13695==by 0x7041A86: (below main) (libc-start.c:310)

Signed-off-by: Michel Dänzer <michel.daen...@amd.com>
---
 randr/rroutput.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/randr/rroutput.c b/randr/rroutput.c
index b0ffedea8..33300e1cc 100644
--- a/randr/rroutput.c
+++ b/randr/rroutput.c
@@ -106,6 +106,7 @@ RROutputCreate(ScreenPtr pScreen,
 output->properties = NULL;
 output->pendingProperties = FALSE;
 output->changed = FALSE;
+output->nonDesktop = FALSE;
 output->devPrivate = devPrivate;
 
 if (!AddResource(output->id, RROutputType, (void *) output))
-- 
2.16.2

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver v3 23/24] xwayland: Cut off upper 32bit of queued vblank delay

2018-03-14 Thread Michel Dänzer
On 2018-03-13 04:00 PM, Roman Gilg wrote:
> This ensures the same behavior as in Present's fake counter and xfree86.
> 
> At the moment clients might do put vblanks too far into the future, because
> the fake vblank code in Present and the xfree86 driver tolerate cut off upper
> 32bit due to an 64 to 32bit conversion. Do this therefore here as well to not
> suddenly regress on Xwayland only.
> 
> The sample client, that triggers this behavior, is the Steam client.
> 
> Signed-off-by: Roman Gilg <subd...@gmail.com>
> ---
>  hw/xwayland/xwayland-present.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
> index f403ff7..300d96f 100644
> --- a/hw/xwayland/xwayland-present.c
> +++ b/hw/xwayland/xwayland-present.c
> @@ -299,6 +299,7 @@ xwl_present_queue_vblank(WindowPtr present_window,
>  {
>  struct xwl_window *xwl_window = xwl_window_of_top(present_window);
>  struct xwl_present_event *event;
> +INT32 delay;
>  
>  if (!xwl_window)
>  return BadMatch;
> @@ -317,7 +318,10 @@ xwl_present_queue_vblank(WindowPtr present_window,
>  event->event_id = event_id;
>  event->present_window = present_window;
>  event->xwl_window = xwl_window;
> -event->target_msc = msc;
> +
> +/* Cut off upper 32bit, copies present_fake_queue_vblank. */
> +delay = (int64_t) (msc - xwl_window->present_msc);
> +event->target_msc = xwl_window->present_msc + delay;

FWIW, I think it would be better to make delay unsigned, and calculate
it something like

delay = max((int64_t) (msc - xwl_window->present_msc), 0);


However, since I've been unable to reproduce the issue you described
with Steam, despite kind of going out of my way to do so, I'm really
reluctant to add this workaround without getting more information about
the problem from someone who can reproduce it. Without understanding the
problem, the workaround might turn out to be too strict or too lenient,
and clients might accidentally become dependent on the workaround.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] dix: don't free() stack memory

2018-03-13 Thread Michel Dänzer
On 2018-03-13 11:56 AM, Eric Engestrom wrote:
> In function ‘doImageText’,
> inlined from ‘ImageText’ at dix/dixfonts.c:1513:5:
> dix/dixfonts.c:1492:9: warning: attempt to free a non-heap object 
> ‘local_closure’ [-Wfree-nonheap-object]
>  free(c);
>  ^
> 
> Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
> ---
>  dix/dixfonts.c | 23 ---
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/dix/dixfonts.c b/dix/dixfonts.c
> index cca92ed2791ccf262017..c48034dd41426b47915d 100644
> --- a/dix/dixfonts.c
> +++ b/dix/dixfonts.c
> @@ -1498,19 +1498,20 @@ int
>  ImageText(ClientPtr client, DrawablePtr pDraw, GC * pGC, int nChars,
>unsigned char *data, int xorg, int yorg, int reqType, XID did)
>  {
> -ITclosureRec local_closure;
> +ITclosureRec *local_closure = malloc(sizeof(*local_closure));
>  
> -local_closure.client = client;
> -local_closure.pDraw = pDraw;
> -local_closure.pGC = pGC;
> -local_closure.nChars = nChars;
> -local_closure.data = data;
> -local_closure.xorg = xorg;
> -local_closure.yorg = yorg;
> -local_closure.reqType = reqType;
> -local_closure.did = did;
> +local_closure->client = client;
> +local_closure->pDraw = pDraw;
> +local_closure->pGC = pGC;
> +local_closure->nChars = nChars;
> +local_closure->data = data;
> +local_closure->xorg = xorg;
> +local_closure->yorg = yorg;
> +local_closure->reqType = reqType;
> +local_closure->did = did;
>  
> -(void) doImageText(client, _closure);
> +(void) doImageText(client, local_closure);
> +free(local_closure);

If the free(c) in the compiler warning above is hit, this is a
double-free, isn't it?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver v2 22/22] xwayland: Guard against very late vblanks

2018-03-07 Thread Michel Dänzer
On 2018-02-28 07:00 PM, Roman Gilg wrote:
> On Wed, Feb 28, 2018 at 6:43 PM, Michel Dänzer <mic...@daenzer.net> wrote:
>> I'm unable to reproduce the issue you described with Steam (without this
>> patch series applied). I'd really like to know where the bogus target
>> MSC values you're seeing are coming from. What are the values of the
>> window_msc parameter and window_priv->msc_offset in
>> present_window_to_crtc_msc?
> 
> You don't have to go that far. I did read out on current master high
> target msc values directly for stuff->target_msc in
> proc_present_pixmap. But this happened only on like two or three
> occasions directly at the startup of the Steam client while hundreds
> of other present_pixmap requests before and after had normal
> stuff->target_msc values.
> 
> So just put a line
> 
> ErrorF("XXX %lu\n", stuff->target_msc);
> 
> in proc_present_pixmap and if you look carefully through the debug
> output there should be one or two very high numbers in comparison to
> all the other values.

As discussed on IRC, I've been unable to reproduce the issue you're
seeing yet, despite trying various things. At this point, the most
likely trigger of your symptoms is that in Mesa's
dri3_handle_present_event, the

 if (draw->recv_sbc > draw->send_sbc)
draw->recv_sbc -= 0x1;

is incorrectly hit, e.g. due to processing events from presentations of
another client/context. We'll need more information from you to validate
that hypothesis.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver] glamor: Restore glamor_fd_from_pixmap and glamor_pixmap_from_fd

2018-03-06 Thread Michel Dänzer
From: Michel Dänzer <michel.daen...@amd.com>

They're part of the 1.20 RC1 ABI, and actually used by external drivers.
Also, requiring drivers which don't support the new functionality in
DRI3 1.2 to switch to the new interfaces seems unreasonable.

Signed-off-by: Michel Dänzer <michel.daen...@amd.com>
---
 glamor/glamor.c | 35 +++
 glamor/glamor.h | 35 +++
 glamor/glamor_egl.c | 22 ++
 3 files changed, 80 insertions(+), 12 deletions(-)

diff --git a/glamor/glamor.c b/glamor/glamor.c
index c7077dd84..acc7de940 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -845,26 +845,18 @@ glamor_fds_from_pixmap(ScreenPtr screen, PixmapPtr 
pixmap, int *fds,
 }
 
 _X_EXPORT int
-glamor_shareable_fd_from_pixmap(ScreenPtr screen,
-PixmapPtr pixmap, CARD16 *stride, CARD32 *size)
+glamor_fd_from_pixmap(ScreenPtr screen,
+  PixmapPtr pixmap, CARD16 *stride, CARD32 *size)
 {
-unsigned orig_usage_hint = pixmap->usage_hint;
 int ret;
 int fds[4];
 uint32_t strides[4], offsets[4];
 uint64_t modifier;
 
-/*
- * The actual difference between a sharable and non sharable buffer
- * is decided 4 call levels deep in glamor_make_pixmap_exportable()
- * based on pixmap->usage_hint == CREATE_PIXMAP_USAGE_SHARED
- * 2 of those calls are also exported API, so we cannot just add a flag.
- */
-pixmap->usage_hint = CREATE_PIXMAP_USAGE_SHARED;
 ret = glamor_fds_from_pixmap(screen, pixmap, fds, strides, offsets,
  );
 
-/* Pixmaps with multi-planes/modifier are not shareable */
+/* Pixmaps with multi-planes/modifier are not supported in this interface 
*/
 if (ret > 1) {
 while (ret > 0)
 close(fds[--ret]);
@@ -875,8 +867,27 @@ glamor_shareable_fd_from_pixmap(ScreenPtr screen,
 *stride = strides[0];
 *size = pixmap->drawable.height * *stride;
 
-pixmap->usage_hint = orig_usage_hint;
+return ret;
+}
+
+_X_EXPORT int
+glamor_shareable_fd_from_pixmap(ScreenPtr screen,
+PixmapPtr pixmap, CARD16 *stride, CARD32 *size)
+{
+unsigned orig_usage_hint = pixmap->usage_hint;
+int ret;
 
+/*
+ * The actual difference between a sharable and non sharable buffer
+ * is decided 4 call levels deep in glamor_make_pixmap_exportable()
+ * based on pixmap->usage_hint == CREATE_PIXMAP_USAGE_SHARED
+ * 2 of those calls are also exported API, so we cannot just add a flag.
+ */
+pixmap->usage_hint = CREATE_PIXMAP_USAGE_SHARED;
+
+ret = glamor_fd_from_pixmap(screen, pixmap, stride, size);
+
+pixmap->usage_hint = orig_usage_hint;
 return ret;
 }
 
diff --git a/glamor/glamor.h b/glamor/glamor.h
index 5475aedbb..7b5676226 100644
--- a/glamor/glamor.h
+++ b/glamor/glamor.h
@@ -183,6 +183,21 @@ extern _X_EXPORT int glamor_fds_from_pixmap(ScreenPtr 
screen,
 uint32_t *strides, uint32_t 
*offsets,
 uint64_t *modifier);
 
+/* @glamor_fd_from_pixmap: Get a dma-buf fd from a pixmap.
+ *
+ * @screen: Current screen pointer.
+ * @pixmap: The pixmap from which we want the fd.
+ * @stride, @size: Pointers to fill the stride and size of the
+ *buffer associated to the fd.
+ *
+ * the pixmap and the buffer associated by the fd will share the same
+ * content.
+ * Returns the fd on success, -1 on error.
+ * */
+extern _X_EXPORT int glamor_fd_from_pixmap(ScreenPtr screen,
+   PixmapPtr pixmap,
+   CARD16 *stride, CARD32 *size);
+
 /* @glamor_shareable_fd_from_pixmap: Get a dma-buf fd suitable for sharing
  *  with other GPUs from a pixmap.
  *
@@ -258,6 +273,26 @@ extern _X_EXPORT PixmapPtr 
glamor_pixmap_from_fds(ScreenPtr screen,
   CARD8 bpp,
   uint64_t modifier);
 
+/* @glamor_pixmap_from_fd: Creates a pixmap to wrap a dma-buf fd.
+ *
+ * @screen: Current screen pointer.
+ * @fd: The dma-buf fd to import.
+ * @width: The width of the buffer.
+ * @height: The height of the buffer.
+ * @stride: The stride of the buffer.
+ * @depth: The depth of the buffer.
+ * @bpp: The bpp of the buffer.
+ *
+ * Returns a valid pixmap if the import succeeded, else NULL.
+ * */
+extern _X_EXPORT PixmapPtr glamor_pixmap_from_fd(ScreenPtr screen,
+ int fd,
+ CARD16 width,
+ CARD16 height,
+ CARD16 stride,
+ CARD8 depth,
+ 

Re: [PATCH xserver v2 00/22] Per window flips in Present with support for them in Xwayland

2018-02-28 Thread Michel Dänzer
On 2018-02-28 05:36 PM, Roman Gilg wrote:
> This revision provides changes requested by Michel Dänzer. In particular
> flips without copies in window flip mode are now possible and clients can
> queue flips on Xwayland.

Thanks for that.

It would be even better if queuing flips worked from the beginning. If
reordering the patches accordingly is difficult, can you split out the
xwl_present_init related parts of patch 19 to a new patch after patch
21? If even that is difficult, leaving it like this is OK.


Unfortunately, I don't have the bandwidth to look at every patch in
detail right now, but apart from the above and my followup to patch 22,
I'm fine with this landing (assuming it actually works :).


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver v2 22/22] xwayland: Guard against very late vblanks

2018-02-28 Thread Michel Dänzer
On 2018-02-28 05:37 PM, Roman Gilg wrote:
> Do not allow queuing events too far into the the future. The result can be a
> presentation freeze until the msc time is reached.
> 
> At the moment clients might do this per accident, because the fake vblank code
> in Present and the xfree86 driver tolerate high 64bit msc values without
> freeze due to an erroneous 64 to 32bit conversion.
> 
> The sample client, that triggers this behavior, is the Steam client.

I'm unable to reproduce the issue you described with Steam (without this
patch series applied). I'd really like to know where the bogus target
MSC values you're seeing are coming from. What are the values of the
window_msc parameter and window_priv->msc_offset in
present_window_to_crtc_msc?


> diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
> index 27bc569..c0bc1f0 100644
> --- a/hw/xwayland/xwayland-present.c
> +++ b/hw/xwayland/xwayland-present.c
> @@ -303,6 +303,11 @@ xwl_present_queue_vblank(WindowPtr present_window,
>  if (!xwl_window)
>  return BadMatch;
>  
> +if (msc > xwl_window->present_msc + 100) {
> +ErrorF("Client queued frame too far in the future: %lu -> %lu\n", 
> xwl_window->present_msc, msc);
> +return BadRequest;
> +}

If we're going this route, let's not use an arbitrary threshold like
100, but the actual one corresponding to the fake vblank code dropping
the most significant bits.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

  1   2   3   4   5   6   7   8   9   10   >