Re: Rework TTMs busy handling
On 2024-01-09 09:34, Christian König wrote: > Am 09.01.24 um 09:14 schrieb Thomas Hellström: >> On Tue, 2024-01-09 at 08:47 +0100, Christian König wrote: >>> >>> I'm trying to make this functionality a bit more useful for years now >>> since we multiple reports that behavior of drivers can be suboptimal >>> when multiple placements be given. >>> >>> So basically instead of hacking around the TTM behavior in the driver >>> once more I've gone ahead and changed the idle/busy placement list >>> into idle/busy placement flags. This not only saves a bunch of code, >>> but also allows setting some placements as fallback which are used if >>> allocating from the preferred ones didn't worked. >> >> I also have some doubts about the naming "idle" vs "busy", since an >> elaborate eviction mechanism would probably at some point want to check >> for gpu idle vs gpu busy, and this might create some confusion moving >> forward for people confusing busy as in memory overcommit with busy as >> in gpu activity. >> >> I can't immediately think of something better, though. > > Yeah, I was wondering about that as well. Especially since I wanted to add > some more flags in the future when for example a bandwidth quota how much > memory can be moved in/out is exceeded. > > Something like phase1, phase2, phase3 etc..., but that's also not very > descriptive either. Maybe something like "desired" vs "fallback"? -- Earthling Michel Dänzer| https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
Re: [Nouveau] [PATCH (set 1) 00/20] Rid W=1 warnings from GPU
On 8/24/23 14:07, Lee Jones wrote: > On Thu, 24 Aug 2023, Jani Nikula wrote: >> On Thu, 24 Aug 2023, Lee Jones wrote: >>> This set is part of a larger effort attempting to clean-up W=1 >>> kernel builds, which are currently overwhelmingly riddled with >>> niggly little warnings. >> >> The next question is, how do we keep it W=1 clean going forward? > > My plan was to fix them all, then move each warning to W=0. > > Arnd recently submitted a set doing just that for a bunch of them. > > https://lore.kernel.org/all/20230811140327.3754597-1-a...@kernel.org/ > > I like to think a bunch of this is built on top of my previous efforts. > > GPU is a particularly tricky though - the warnings seem to come in faster > than I can squash them. Maybe the maintainers can find a way to test > new patches on merge? One approach for this which has proved effective in Mesa and other projects is to make warnings fatal in CI which must pass for any changes to be merged. There is ongoing work toward introducing this for the DRM subsystem, using gitlab.freedesktop.org CI. -- Earthling Michel Dänzer| https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
Re: [Nouveau] [PATCH] Change the meaning of the fields in the ttm_place structure from pfn to bytes
On 3/3/23 08:16, Somalapuram Amaranath wrote:
> Change the ttm_place structure member fpfn, lpfn, mem_type to
> res_start, res_end, res_type.
> Change the unsigned to u64.
> Fix the dependence in all the DRM drivers and
> clean up PAGE_SHIFT operation.
>
> Signed-off-by: Somalapuram Amaranath
>
> [...]
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index 44367f03316f..5b5104e724e3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -131,11 +131,12 @@ static int amdgpu_gtt_mgr_new(struct
> ttm_resource_manager *man,
> goto err_free;
> }
>
> - if (place->lpfn) {
> + if (place->res_end) {
> spin_lock(&mgr->lock);
> r = drm_mm_insert_node_in_range(&mgr->mm, &node->mm_nodes[0],
> - num_pages, tbo->page_alignment,
> - 0, place->fpfn, place->lpfn,
> + num_pages, tbo->page_alignment,
> 0,
> + place->res_start << PAGE_SHIFT,
> + place->res_end << PAGE_SHIFT,
> DRM_MM_INSERT_BEST);
This should be >> or no shift instead of <<, shouldn't it? Multiplying a value
in bytes by the page size doesn't make sense.
I didn't check the rest of the patch in detail, but it's easy introduce subtle
regressions with this kind of change. It'll require a lot of review & testing
scrutiny.
--
Earthling Michel Dänzer| https://redhat.com
Libre software enthusiast | Mesa and Xwayland developer
Re: [Nouveau] [RFC] Deprecate AGP GART support for Radeon/Nouveau/TTM
On 2020-05-22 12:40 p.m., Christian König wrote: > Am 20.05.20 um 18:25 schrieb Michel Dänzer: >> On 2020-05-20 4:43 p.m., Christian König wrote: >>> Am 13.05.20 um 13:03 schrieb Christian König: >>>> Unfortunately AGP is still to widely used as we could just drop >>>> support for using its GART. >>>> >>>> Not using the AGP GART also doesn't mean a loss in functionality since >>>> drivers will just fallback to the driver specific PCI GART. >>>> >>>> For now just deprecate the code and don't enable the AGP GART in TTM >>>> even when general AGP support is available. >>> So I've used an ancient system (32bit) to setup a test box for this. >>> >>> >>> The first GPU I could test is an RV280 (Radeon 9200 PRO) which is easily >>> 15 years old. >>> >>> What happens in AGP mode is that glxgears shows artifacts during >>> rendering on this system. >>> >>> In PCI mode those rendering artifacts are gone and glxgears seems to >>> draw everything correctly now. >>> >>> Performance is obviously not comparable, cause in AGP we don't render >>> all triangles correctly. >>> >>> >>> The second GPU I could test is an RV630 PRO (Radeon HD 2600 PRO AGP) >>> which is more than 10 years old. >>> >>> As far as I can tell this one works in both AGP and PCIe mode perfectly >>> fine. >>> >>> Since this is only a 32bit system I couldn't really test any OpenGL game >>> that well. >>> >>> But for glxgears switching from AGP to PCIe mode seems to result in a >>> roughly 5% performance drop. >>> >>> The surprising reason for this is not the better TLB performance, but >>> the lack of USWC support for the PCIe GART in radeon. >> I suspect the main reason it's only 5% is that PCIe GART page tables are >> stored in VRAM, so they don't need to be fetched across the PCIe link >> (and presumably it has more than one TLB entry as well). The difference >> is much bigger with native AGP ASICs with PCI GART. > > Do you have some hardware you could give that a try on? As I mentioned before, I tested this many times on my AGP PowerBooks back in the day. The result was always a similar, big hit with PCI GART vs AGP (even just 1x). I haven't seen any reason to believe this has changed. > While I agree that it means a performance regression, this is a rather > high motivation to go ahead with at least the first patch. I totally agree with the benefits, I just want everyone to be honest and clear about the performance hit with native AGP Radeons, which already have very weak performance by today's standards even with AGP. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [RFC] Deprecate AGP GART support for Radeon/Nouveau/TTM
On 2020-05-20 4:43 p.m., Christian König wrote: > Am 13.05.20 um 13:03 schrieb Christian König: >> Unfortunately AGP is still to widely used as we could just drop >> support for using its GART. >> >> Not using the AGP GART also doesn't mean a loss in functionality since >> drivers will just fallback to the driver specific PCI GART. >> >> For now just deprecate the code and don't enable the AGP GART in TTM >> even when general AGP support is available. > > So I've used an ancient system (32bit) to setup a test box for this. > > > The first GPU I could test is an RV280 (Radeon 9200 PRO) which is easily > 15 years old. > > What happens in AGP mode is that glxgears shows artifacts during > rendering on this system. > > In PCI mode those rendering artifacts are gone and glxgears seems to > draw everything correctly now. > > Performance is obviously not comparable, cause in AGP we don't render > all triangles correctly. > > > The second GPU I could test is an RV630 PRO (Radeon HD 2600 PRO AGP) > which is more than 10 years old. > > As far as I can tell this one works in both AGP and PCIe mode perfectly > fine. > > Since this is only a 32bit system I couldn't really test any OpenGL game > that well. > > But for glxgears switching from AGP to PCIe mode seems to result in a > roughly 5% performance drop. > > The surprising reason for this is not the better TLB performance, but > the lack of USWC support for the PCIe GART in radeon. I suspect the main reason it's only 5% is that PCIe GART page tables are stored in VRAM, so they don't need to be fetched across the PCIe link (and presumably it has more than one TLB entry as well). The difference is much bigger with native AGP ASICs with PCI GART. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 1/3] drm/radeon: remove AGP support
On 2020-05-13 9:46 a.m., Christian König wrote: > Am 12.05.20 um 23:12 schrieb Alex Deucher: >> On Tue, May 12, 2020 at 4:52 PM Roy Spliet wrote: >>> >>> I'll volunteer to be the one asking: how big is this performance >>> difference? Have any benchmarks been run before and after removal of AGP >>> GART code on affected nouveau/radeon systems? Or is this code being >>> dropped _just_ because it's cumbersome, with no regard for metrics that >>> determine the value of AGP GART support? >>> >> I don't think anyone has any solid numbers, just anecdotal from >> memory. I certainly don't have any functional AGP systems at this >> point. It's mostly just cumbersome and would allow us to clean ttm >> and probably improve stability at the same time. At least on the >> radeon side, the only native AGP cards were r1xx, r2xx, and some of >> the early r3xx boards. Once we switched to pcie mid-way through r3xx, >> everything was native pcie and the AGP cards used a pcie to AGP bridge >> chip so they had a decent on chip MMU. Those older cards topped out >> at maybe 32 or 64 MB of vram, so they are going to be hard pressed to >> deal with modern desktops anyway. No idea what sort of GART >> capabilities NV AGP hardware at this time had. > > I could only test with an old x86 Mac and an r3xx generation hw and in > this case making the switch didn't had any noticeable effect at all. > > But I didn't do more than playing around with the desktop effects and > playing a video. Yeah, that's not enough to see a difference. Try an OpenGL game, or even just glxgears. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 1/3] drm/radeon: remove AGP support
On 2020-05-11 10:12 p.m., Alex Deucher wrote: > On Mon, May 11, 2020 at 1:17 PM Christian König > wrote: >> >> AGP is deprecated for 10+ years now and not used any more on modern hardware. >> >> Old hardware should continue to work in PCI mode. > > Might want to clarify that there is no loss of functionality here. > Something like: > > "There is no loss of functionality here. GPUs will continue to > function. This just drops the use of the AGP MMU in the chipset in > favor of the MMU on the device which has proven to be much more > reliable. Due to its unreliability, AGP support has been disabled on > PowerPC for years already so there is no change on PowerPC." There's a difference between something being disabled by default or not being available at all. We may decide it's worth it anyway, but let's do it based on facts. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 0/5] Fix deadlock on runtime suspend in DRM drivers
On 2018-02-14 03:08 PM, Sean Paul wrote: > On Wed, Feb 14, 2018 at 10:26:35AM +0100, Maarten Lankhorst wrote: >> Op 14-02-18 om 09:46 schreef Lukas Wunner: >>> Dear drm-misc maintainers, >>> >>> On Sun, Feb 11, 2018 at 10:38:28AM +0100, Lukas Wunner wrote: >>>> Fix a deadlock on hybrid graphics laptops that's been present since 2013: >>> This series has been reviewed, consent has been expressed by the most >>> interested parties, patch [1/5] which touches files outside drivers/gpu >>> has been acked and I've just out a v2 addressing the only objection >>> raised. My plan is thus to wait another two days for comments and, >>> barring further objections, push to drm-misc this weekend. >>> >>> However I'm struggling with the decision whether to push to next or >>> fixes. The series is marked for stable, however the number of >>> affected machines is limited and for an issue that's been present >>> for 5 years it probably doesn't matter if it soaks another two months >>> in linux-next befor it gets backported. Hence I tend to err on the >>> side of caution and push to next, however a case could be made that >>> fixes is more appropriate. >>> >>> I'm lacking experience making such decisions and would be interested >>> to learn how you'd handle this. >>> >>> Thanks, >>> >>> Lukas >> >> I would say fixes, it doesn't look particularly scary. :) > > Agreed. If it's good enough for stable, it's good enough for -fixes! It's not that simple, is it? Fast-tracking patches (some of which appear to be untested) to stable without an immediate cause for urgency seems risky to me. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] nouveau. swiotlb: coherent allocation failed for device 0000:01:00.0 size=2097152
On 2017-12-19 11:37 AM, Michel Dänzer wrote: > On 2017-12-18 08:01 PM, Tobias Klausmann wrote: >> On 12/18/17 7:06 PM, Mike Galbraith wrote: >>> Greetings, >>> >>> Kernel bound workloads seem to trigger the below for whatever reason. >>> I only see this when beating up NFS. There was a kworker wakeup >>> latency issue, but with a bandaid applied to fix that up, I can still >>> trigger this. >> >> >> Hi, >> >> i have seen this one as well with my system, but i could not find an >> easy way to trigger it for bisecting purpose. If you can trigger it >> conveniently, a bisect would be nice! > > I'm seeing this (with the amdgpu and radeon drivers) when restic takes a > backup, creating memory pressure. I happen to have just finished > bisecting, the result is: > > 648bc3574716400acc06f99915815f80d9563783 is the first bad commit > commit 648bc3574716400acc06f99915815f80d9563783 > Author: Christian König > Date: Thu Jul 6 09:59:43 2017 +0200 > > drm/ttm: add transparent huge page support for DMA allocations v2 > > Try to allocate huge pages when it makes sense. > > v2: fix comment and use ifdef > > BTW, I haven't noticed any bad effects other than the dmesg splats, so maybe it's just noise about transient failures for which there is a proper fallback in place. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] nouveau. swiotlb: coherent allocation failed for device 0000:01:00.0 size=2097152
On 2017-12-18 08:01 PM, Tobias Klausmann wrote: > On 12/18/17 7:06 PM, Mike Galbraith wrote: >> Greetings, >> >> Kernel bound workloads seem to trigger the below for whatever reason. >> I only see this when beating up NFS. There was a kworker wakeup >> latency issue, but with a bandaid applied to fix that up, I can still >> trigger this. > > > Hi, > > i have seen this one as well with my system, but i could not find an > easy way to trigger it for bisecting purpose. If you can trigger it > conveniently, a bisect would be nice! I'm seeing this (with the amdgpu and radeon drivers) when restic takes a backup, creating memory pressure. I happen to have just finished bisecting, the result is: 648bc3574716400acc06f99915815f80d9563783 is the first bad commit commit 648bc3574716400acc06f99915815f80d9563783 Author: Christian König Date: Thu Jul 6 09:59:43 2017 +0200 drm/ttm: add transparent huge page support for DMA allocations v2 Try to allocate huge pages when it makes sense. v2: fix comment and use ifdef -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH xf86-video-amdgpu] Adapt to PixmapDirtyUpdateRec::src being a DrawablePtr
On 14/07/17 11:24 AM, Ilia Mirkin wrote: > On Thu, Jul 13, 2017 at 10:14 PM, Michel Dänzer wrote: >> On 13/07/17 09:31 PM, Ilia Mirkin wrote: >>> On Thu, Jul 13, 2017 at 4:27 AM, Michel Dänzer wrote: >>>> On 18/04/17 07:07 PM, Michel Dänzer wrote: >>>>> From: Michel Dänzer >>>>> >>>>> Signed-off-by: Michel Dänzer >>>>> --- >>>>> >>>>> Chris / Ilia / Ben, this should be manageable for the intel/nouveau >>>>> drivers, right? >>>> >>>> Any feedback, guys? >>>> >>>> I want to push the xserver change soon-ish. I could probably come up >>>> with a corresponding patch for nouveau in the worst case, but I'm afraid >>>> not for intel. >>> >>> Sorry, I'm not 1000% clear on what this patch is doing. >> >> 100% should suffice. ;) >> >>> Is it literally just a type change from A to B and so code has to be fixed >>> up for the new API? >> >> That's basically it, corresponding to >> https://patchwork.freedesktop.org/patch/150938/ . > > From a brief glance at nouveau, it never uses PixmapDirtyUpdate, > except in src/nv_driver.c where I don't think the code would need to > be updated. > > https://cgit.freedesktop.org/nouveau/xf86-video-nouveau/tree/src/nv_driver.c#n552 There's also PixmapStart/StopDirtyTracking, though it looks like those might happen to work as well at runtime, if you don't mind the compiler warnings. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH xf86-video-amdgpu] Adapt to PixmapDirtyUpdateRec::src being a DrawablePtr
On 13/07/17 09:31 PM, Ilia Mirkin wrote: > On Thu, Jul 13, 2017 at 4:27 AM, Michel Dänzer wrote: >> On 18/04/17 07:07 PM, Michel Dänzer wrote: >>> From: Michel Dänzer >>> >>> Signed-off-by: Michel Dänzer >>> --- >>> >>> Chris / Ilia / Ben, this should be manageable for the intel/nouveau >>> drivers, right? >> >> Any feedback, guys? >> >> I want to push the xserver change soon-ish. I could probably come up >> with a corresponding patch for nouveau in the worst case, but I'm afraid >> not for intel. > > Sorry, I'm not 1000% clear on what this patch is doing. 100% should suffice. ;) > Is it literally just a type change from A to B and so code has to be fixed > up for the new API? That's basically it, corresponding to https://patchwork.freedesktop.org/patch/150938/ . -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH xf86-video-amdgpu] Adapt to PixmapDirtyUpdateRec::src being a DrawablePtr
On 18/04/17 07:07 PM, Michel Dänzer wrote: > From: Michel Dänzer > > Signed-off-by: Michel Dänzer > --- > > Chris / Ilia / Ben, this should be manageable for the intel/nouveau > drivers, right? Any feedback, guys? I want to push the xserver change soon-ish. I could probably come up with a corresponding patch for nouveau in the worst case, but I'm afraid not for intel. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
On 21/06/17 05:14 PM, Daniel Vetter wrote:
> On Wed, Jun 21, 2017 at 04:59:31PM +0900, Michel Dänzer wrote:
>> On 21/06/17 04:38 PM, Daniel Vetter wrote:
>>> On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote:
>>>> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
>>>> totally obsolete.
>>>>
>>>> I think the gamma_store can end up invalid on error. But the way I read
>>>> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
>>>> this pesky legacy fbdev stuff be any better?
>>>>
>>>> drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However,
>>>> it saves it to the gamma_store which should already be up to date with what
>>>> .gamma_get would return and is thus a nop. So, zap it.
>>>
>>> Removing drm_fb_helper_save_lut_atomic should be a separate patch I
>>> think.
>>>
>>>> Signed-off-by: Peter Rosin
>>
>> [...]
>>
>>>> @@ -1167,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct
>>>> drm_fb_helper *fb_helper,
>>>> }
>>>> EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
>>>>
>>>> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
>>>> - u16 blue, u16 regno, struct fb_info *info)
>>>> -{
>>>> - struct drm_fb_helper *fb_helper = info->par;
>>>> - struct drm_framebuffer *fb = fb_helper->fb;
>>>> -
>>>> - if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
>>>
>>> This case here seems gone, and it was also the pièce de résistance when I
>>> tried tackling fbdev lut support. As far as I understand this stuff we do
>>> not support FB_VISUAL_TRUECOLOR palette, and all that bitshifting here is
>>> pointless. But I'm honestly not really clear.
>>>
>>> I think removing this case should also be a separate patch, and I'd very
>>> much prefer that someone with some fbdev-clue would ack it.
>>
>> No need for anyone with fbdev-clue, just run fbset -i. :)
Adding Bartlomiej and the linux-fbdev list, just in case I'm wrong below.
>> fbcon uses a TRUECOLOR visual at depths > 8.
>
> Then I understand even less what exactly this code does ... Should we keep
> it?
I think we need it. It updates the entries of info->pseudo_palette,
which is kind of a pseudocolor palette mapping 16 indices to pixel
values to write to the framebuffer.
If the setcolreg hook is removed, the setcmap hook needs to do this instead.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
On 21/06/17 04:38 PM, Daniel Vetter wrote:
> On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote:
>> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
>> totally obsolete.
>>
>> I think the gamma_store can end up invalid on error. But the way I read
>> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
>> this pesky legacy fbdev stuff be any better?
>>
>> drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However,
>> it saves it to the gamma_store which should already be up to date with what
>> .gamma_get would return and is thus a nop. So, zap it.
>
> Removing drm_fb_helper_save_lut_atomic should be a separate patch I
> think.
>
>> Signed-off-by: Peter Rosin
[...]
>> @@ -1167,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct
>> drm_fb_helper *fb_helper,
>> }
>> EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
>>
>> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
>> - u16 blue, u16 regno, struct fb_info *info)
>> -{
>> -struct drm_fb_helper *fb_helper = info->par;
>> -struct drm_framebuffer *fb = fb_helper->fb;
>> -
>> -if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
>
> This case here seems gone, and it was also the pièce de résistance when I
> tried tackling fbdev lut support. As far as I understand this stuff we do
> not support FB_VISUAL_TRUECOLOR palette, and all that bitshifting here is
> pointless. But I'm honestly not really clear.
>
> I think removing this case should also be a separate patch, and I'd very
> much prefer that someone with some fbdev-clue would ack it.
No need for anyone with fbdev-clue, just run fbset -i. :) fbcon uses a
TRUECOLOR visual at depths > 8.
> It's a pre-existing bug, but should we also try to restore the fbdev lut
> in drm_fb_helper_restore_fbdev_mode_unlocked()? Would be yet another bug,
> but might be relevant for your use-case. Just try to run both an fbdev
> application and some kms-native thing, and then SIGKILL the native kms
> app.
>
> But since pre-existing not really required, and probably too much effort.
I suspect something like that indeed needs to be done though. E.g.
killing Xorg results in fbcon continuing to use the LUT set by Xorg.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH xserver] Make PixmapDirtyUpdateRec::src a DrawablePtr
From: Michel Dänzer
This allows making the master screen's pixmap_dirty_list entries
explicitly reflect that we're now tracking the root window instead of
the screen pixmap, in order to allow Present page flipping on master
outputs while there are active slave outputs.
Define HAS_DIRTYTRACKING_DRAWABLE_SRC for drivers to check, but leave
HAS_DIRTYTRACKING_ROTATION defined as well to make things slightly
easier for drivers.
Signed-off-by: Michel Dänzer
---
The Start/StopFlippingPixmapTracking related changes are only compile
tested. The rest is smoke tested with the modesetting, amdgpu and radeon
drivers.
dix/pixmap.c | 24 +++-
hw/xfree86/drivers/modesetting/driver.c | 10 +-
hw/xfree86/drivers/modesetting/drmmode_display.c | 5 +++--
hw/xfree86/drivers/modesetting/drmmode_display.h | 2 +-
include/pixmap.h | 5 +++--
include/pixmapstr.h | 3 ++-
include/scrnintstr.h | 6 +++---
randr/randrstr.h | 2 +-
randr/rrcrtc.c | 20 +---
9 files changed, 38 insertions(+), 39 deletions(-)
diff --git a/dix/pixmap.c b/dix/pixmap.c
index b67a2e8a6..81ac5e2d8 100644
--- a/dix/pixmap.c
+++ b/dix/pixmap.c
@@ -181,12 +181,12 @@ PixmapDirtyDamageDestroy(DamagePtr damage, void *closure)
}
Bool
-PixmapStartDirtyTracking(PixmapPtr src,
+PixmapStartDirtyTracking(DrawablePtr src,
PixmapPtr slave_dst,
int x, int y, int dst_x, int dst_y,
Rotation rotation)
{
-ScreenPtr screen = src->drawable.pScreen;
+ScreenPtr screen = src->pScreen;
PixmapDirtyUpdatePtr dirty_update;
RegionPtr damageregion;
RegionRec dstregion;
@@ -204,8 +204,7 @@ PixmapStartDirtyTracking(PixmapPtr src,
dirty_update->dst_y = dst_y;
dirty_update->rotation = rotation;
dirty_update->damage = DamageCreate(NULL, PixmapDirtyDamageDestroy,
-DamageReportNone,
-TRUE, src->drawable.pScreen,
+DamageReportNone, TRUE, screen,
dirty_update);
if (rotation != RR_Rotate_0) {
@@ -241,16 +240,15 @@ PixmapStartDirtyTracking(PixmapPtr src,
RegionUnion(damageregion, damageregion, &dstregion);
RegionUninit(&dstregion);
-DamageRegister(screen->root ? &screen->root->drawable : &src->drawable,
- dirty_update->damage);
+DamageRegister(src, dirty_update->damage);
xorg_list_add(&dirty_update->ent, &screen->pixmap_dirty_list);
return TRUE;
}
Bool
-PixmapStopDirtyTracking(PixmapPtr src, PixmapPtr slave_dst)
+PixmapStopDirtyTracking(DrawablePtr src, PixmapPtr slave_dst)
{
-ScreenPtr screen = src->drawable.pScreen;
+ScreenPtr screen = src->pScreen;
PixmapDirtyUpdatePtr ent, safe;
xorg_list_for_each_entry_safe(ent, safe, &screen->pixmap_dirty_list, ent) {
@@ -269,8 +267,8 @@ PixmapDirtyCopyArea(PixmapPtr dst,
PixmapDirtyUpdatePtr dirty,
RegionPtr dirty_region)
{
-ScreenPtr pScreen = dirty->src->drawable.pScreen;
-DrawablePtr src = pScreen->root ? &pScreen->root->drawable :
&dirty->src->drawable;
+DrawablePtr src = dirty->src;
+ScreenPtr pScreen = src->pScreen;
int n;
BoxPtr b;
GCPtr pGC;
@@ -309,7 +307,7 @@ PixmapDirtyCompositeRotate(PixmapPtr dst_pixmap,
PixmapDirtyUpdatePtr dirty,
RegionPtr dirty_region)
{
-ScreenPtr pScreen = dirty->src->drawable.pScreen;
+ScreenPtr pScreen = dirty->src->pScreen;
PictFormatPtr format = PictureWindowFormat(pScreen->root);
PicturePtr src, dst;
XID include_inferiors = IncludeInferiors;
@@ -318,7 +316,7 @@ PixmapDirtyCompositeRotate(PixmapPtr dst_pixmap,
int error;
src = CreatePicture(None,
-&pScreen->root->drawable,
+dirty->src,
format,
CPSubwindowMode,
&include_inferiors, serverClient, &error);
@@ -367,7 +365,7 @@ PixmapDirtyCompositeRotate(PixmapPtr dst_pixmap,
*/
Bool PixmapSyncDirtyHelper(PixmapDirtyUpdatePtr dirty)
{
-ScreenPtr pScreen = dirty->src->drawable.pScreen;
+ScreenPtr pScreen = dirty->src->pScreen;
RegionPtr region = DamageRegion(dirty->damage);
PixmapPtr dst;
SourceValidateProcPtr SourceValidate;
diff --git a/hw/xfree86/drivers/modesetting/driver.c
b/hw/xfree86/drivers/modesetting/driver.c
index d7030e5c2..8ce51a502 100644
---
[Nouveau] [PATCH xf86-video-amdgpu] Adapt to PixmapDirtyUpdateRec::src being a DrawablePtr
From: Michel Dänzer
Signed-off-by: Michel Dänzer
---
Chris / Ilia / Ben, this should be manageable for the intel/nouveau
drivers, right?
src/amdgpu_drv.h | 26 ++
src/amdgpu_kms.c | 18 +-
src/drmmode_display.c | 8 ++--
3 files changed, 41 insertions(+), 11 deletions(-)
diff --git a/src/amdgpu_drv.h b/src/amdgpu_drv.h
index e5c44dc36..9e088e71a 100644
--- a/src/amdgpu_drv.h
+++ b/src/amdgpu_drv.h
@@ -170,6 +170,32 @@ typedef enum {
#define AMDGPU_PIXMAP_SHARING 1
#define amdgpu_is_gpu_screen(screen) (screen)->isGPU
#define amdgpu_is_gpu_scrn(scrn) (scrn)->is_gpu
+
+static inline ScreenPtr
+amdgpu_dirty_master(PixmapDirtyUpdatePtr dirty)
+{
+#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
+ ScreenPtr screen = dirty->src->pScreen;
+#else
+ ScreenPtr screen = dirty->src->drawable.pScreen;
+#endif
+
+ if (screen->current_master)
+ return screen->current_master;
+
+ return screen;
+}
+
+static inline Bool
+amdgpu_dirty_src_equals(PixmapDirtyUpdatePtr dirty, PixmapPtr pixmap)
+{
+#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
+ return dirty->src == &pixmap->drawable;
+#else
+ return dirty->src == pixmap;
+#endif
+}
+
#else
#define amdgpu_is_gpu_screen(screen) 0
#define amdgpu_is_gpu_scrn(scrn) 0
diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index 0182cbe0a..29d3d076f 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -448,7 +448,7 @@ dirty_region(PixmapDirtyUpdatePtr dirty)
static void
redisplay_dirty(PixmapDirtyUpdatePtr dirty, RegionPtr region)
{
- ScrnInfoPtr scrn = xf86ScreenToScrn(dirty->src->drawable.pScreen);
+ ScrnInfoPtr scrn = xf86ScreenToScrn(dirty->slave_dst->drawable.pScreen);
if (RegionNil(region))
goto out;
@@ -481,12 +481,12 @@ amdgpu_prime_scanout_update_abort(xf86CrtcPtr crtc, void
*event_data)
void
amdgpu_sync_shared_pixmap(PixmapDirtyUpdatePtr dirty)
{
- ScreenPtr master_screen = dirty->src->master_pixmap->drawable.pScreen;
+ ScreenPtr master_screen = amdgpu_dirty_master(dirty);
PixmapDirtyUpdatePtr ent;
RegionPtr region;
xorg_list_for_each_entry(ent, &master_screen->pixmap_dirty_list, ent) {
- if (ent->slave_dst != dirty->src)
+ if (!amdgpu_dirty_src_equals(dirty, ent->slave_dst))
continue;
region = dirty_region(ent);
@@ -501,7 +501,7 @@ amdgpu_sync_shared_pixmap(PixmapDirtyUpdatePtr dirty)
static Bool
master_has_sync_shared_pixmap(ScrnInfoPtr scrn, PixmapDirtyUpdatePtr dirty)
{
- ScreenPtr master_screen = dirty->src->master_pixmap->drawable.pScreen;
+ ScreenPtr master_screen = amdgpu_dirty_master(dirty);
return master_screen->SyncSharedPixmap != NULL;
}
@@ -517,7 +517,7 @@ slave_has_sync_shared_pixmap(ScrnInfoPtr scrn,
PixmapDirtyUpdatePtr dirty)
static void
call_sync_shared_pixmap(PixmapDirtyUpdatePtr dirty)
{
- ScreenPtr master_screen = dirty->src->master_pixmap->drawable.pScreen;
+ ScreenPtr master_screen = amdgpu_dirty_master(dirty);
master_screen->SyncSharedPixmap(dirty);
}
@@ -527,7 +527,7 @@ call_sync_shared_pixmap(PixmapDirtyUpdatePtr dirty)
static Bool
master_has_sync_shared_pixmap(ScrnInfoPtr scrn, PixmapDirtyUpdatePtr dirty)
{
- ScrnInfoPtr master_scrn =
xf86ScreenToScrn(dirty->src->master_pixmap->drawable.pScreen);
+ ScrnInfoPtr master_scrn = xf86ScreenToScrn(amdgpu_dirty_master(dirty));
return master_scrn->driverName == scrn->driverName;
}
@@ -581,7 +581,7 @@ amdgpu_prime_scanout_do_update(xf86CrtcPtr crtc, unsigned
scanout_id)
Bool ret = FALSE;
xorg_list_for_each_entry(dirty, &screen->pixmap_dirty_list, ent) {
- if (dirty->src == scanoutpix && dirty->slave_dst ==
+ if (amdgpu_dirty_src_equals(dirty, scanoutpix) &&
dirty->slave_dst ==
drmmode_crtc->scanout[scanout_id ^
drmmode_crtc->tear_free].pixmap) {
RegionPtr region;
@@ -738,10 +738,10 @@ amdgpu_dirty_update(ScrnInfoPtr scrn)
PixmapDirtyUpdatePtr region_ent = ent;
if (master_has_sync_shared_pixmap(scrn, ent)) {
- ScreenPtr master_screen =
ent->src->master_pixmap->drawable.pScreen;
+ ScreenPtr master_screen =
amdgpu_dirty_master(ent);
xorg_list_for_each_entry(region_ent,
&master_screen->pixmap_dirty_list, ent) {
- if (region_ent->slave_dst == ent->src)
+ if (amdgpu_dirty_src_equals(ent,
region_ent->slave_dst))
break;
Re: [Nouveau] [PATCH 3/4] drm/amd/display: Switch to using atomic_helper for flip.
On 19/01/17 07:18 AM, Grodzovsky, Andrey wrote:
>> From: Michel Dänzer [mailto:mic...@daenzer.net]
>> On 17/01/17 07:16 AM, Laurent Pinchart wrote:
>>> On Monday 16 Jan 2017 10:44:57 Andrey Grodzovsky wrote:
>>>> Change-Id: Iad3e0b9b3546e4e4dc79be9233daf4fe4dba83e0
>>>> Signed-off-by: Andrey Grodzovsky
>>>> ---
>>>> .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c| 92 ++-
>> ---
>>>> 1 file changed, 6 insertions(+), 86 deletions(-)
>>>>
>>>> diff --git
>> a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
>>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
>> index
>>>> a443b70..d4664bf 100644
>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
>>>> @@ -1060,83 +1060,6 @@ static int dm_crtc_funcs_atomic_set_property(
>>>>return 0;
>>>> }
>>>>
>>>> -
>>>> -static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc,
>>>> - struct drm_framebuffer *fb,
>>>> - struct drm_pending_vblank_event *event,
>>>> - uint32_t flags)
>>>> -{
>>>> - struct drm_plane *plane = crtc->primary;
>>>> - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
>>>> - struct drm_atomic_state *state;
>>>> - struct drm_plane_state *plane_state;
>>>> - struct drm_crtc_state *crtc_state;
>>>> - int ret = 0;
>>>> -
>>>> - state = drm_atomic_state_alloc(plane->dev);
>>>> - if (!state)
>>>> - return -ENOMEM;
>>>> -
>>>> - ret = drm_crtc_vblank_get(crtc);
>>>
>>> The DRM core's atomic page flip helper doesn't get/put vblank. Have
>>> you double-checked that removing them isn't a problem ?
>>
>> This patch makes the amdgpu DM code use the page_flip_target hook.
>> drm_mode_page_flip_ioctl calls drm_crtc_vblank_get before the
>> page_flip_target hook.
>>
>> You're right though that the fact that drm_atomic_helper_page_flip doesn't
>> call drm_crtc_vblank_get is a bit alarming. From the
>> DRM_IOCTL_MODE_PAGE_FLIP POV, drm_crtc_vblank_get must be called
>> when userspace calls the ioctl (either by drm_mode_page_flip_ioctl or the
>> page_flip hook implementation), and drm_crtc_vblank_put must be called
>> when the flip completes and the event is sent to userspace. How is this
>> supposed to be handled with atomic?
>
> First let me ask you - why we have to enable vlbank as part of pflip,
> I understand why it has to be enbbled in WAIT_FOR_VBLANK IOCTL
> but not sure about PFLIP IOCTL.
It's required for the vblank sequence counter to be accurate in the page
flip completion event sent to userspace.
> IMHO in atomic as before in legacy page_flip, it's up to the driver
> implementation in atomic_commit hook to manage vblank ISR on,off.
When using the page_flip hook, it's all up to the implementation of that
hook. When using the page_flip_target hook, drm_mode_page_flip_ioctl
calls vblank_get, so the hook implementation must make sure that a
matching vblank_put call is made when the flip completes.
>> Andrey, did you check via code audit and/or testing that the vblank
>> reference count is still balanced after this change?
>>
> With the page_flip_target yes, if I switch back to page_flip hook then
> vblank ISR never gets enabled on FLIP IPCTL and then I see warning in DAL's
> pflip done IRQ handler from vblank_put which is obvious,
For drivers using the atomic_helpers for flip, maybe there should be (or
might be already) a corresponding helper which deals with things like
this when the flip completes, so drivers don't have to call vblank_put
and such.
Anyway, it sounds like the vblank_get/puts are balanced with this patch
for now.
> which BTW didn't impact the flips, I still was able to run glxgears
> in vsync mode.
I don't think glxgears relies on the vblank sequence numbers being accurate.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 3/4] drm/amd/display: Switch to using atomic_helper for flip.
On 17/01/17 07:16 AM, Laurent Pinchart wrote:
> On Monday 16 Jan 2017 10:44:57 Andrey Grodzovsky wrote:
>> Change-Id: Iad3e0b9b3546e4e4dc79be9233daf4fe4dba83e0
>> Signed-off-by: Andrey Grodzovsky
>> ---
>> .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c| 92 ++
>> 1 file changed, 6 insertions(+), 86 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c index
>> a443b70..d4664bf 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
>> @@ -1060,83 +1060,6 @@ static int dm_crtc_funcs_atomic_set_property(
>> return 0;
>> }
>>
>> -
>> -static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc,
>> -struct drm_framebuffer *fb,
>> -struct drm_pending_vblank_event *event,
>> -uint32_t flags)
>> -{
>> -struct drm_plane *plane = crtc->primary;
>> -struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
>> -struct drm_atomic_state *state;
>> -struct drm_plane_state *plane_state;
>> -struct drm_crtc_state *crtc_state;
>> -int ret = 0;
>> -
>> -state = drm_atomic_state_alloc(plane->dev);
>> -if (!state)
>> -return -ENOMEM;
>> -
>> -ret = drm_crtc_vblank_get(crtc);
>
> The DRM core's atomic page flip helper doesn't get/put vblank. Have you
> double-checked that removing them isn't a problem ?
This patch makes the amdgpu DM code use the page_flip_target hook.
drm_mode_page_flip_ioctl calls drm_crtc_vblank_get before the
page_flip_target hook.
You're right though that the fact that drm_atomic_helper_page_flip
doesn't call drm_crtc_vblank_get is a bit alarming. From the
DRM_IOCTL_MODE_PAGE_FLIP POV, drm_crtc_vblank_get must be called when
userspace calls the ioctl (either by drm_mode_page_flip_ioctl or the
page_flip hook implementation), and drm_crtc_vblank_put must be called
when the flip completes and the event is sent to userspace. How is this
supposed to be handled with atomic?
Andrey, did you check via code audit and/or testing that the vblank
reference count is still balanced after this change?
>> @@ -3143,8 +3064,7 @@ int amdgpu_dm_atomic_check(struct drm_device *dev,
>> * 1. This commit is not a page flip.
>> * 2. This commit is a page flip, and targets are
> created.
>> */
>> -if (!page_flip_needed(plane_state, old_plane_state,
>> - true) ||
>> +if (!page_flip_needed(plane_state, old_plane_state,
> true) ||
>
> This seems to be an unrelated change.
Yeah, such whitespace-only changes should be dropped.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] 4.9-rc7 nouveau fails on arm64 64k page kernel but works with 4k
On 07/12/16 06:39 PM, Alexandre Courbot wrote: > On Fri, Dec 2, 2016 at 12:23 PM, Ilia Mirkin wrote: >> That's right -- nouveau currently requires 4k page sizes to work. This is a >> software limitation, not a hardware one though. > > Looking at the trace I wonder - is the limitation in Nouveau or in TTM? Nouveau. Non-4K page sizes work fine with radeon (and presumably amdgpu). -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events
On 31.10.2015 06:55, Daniel Vetter wrote: > Apparently pre-nv50 pageflip events happen before the actual vblank > period. Therefore that functionality got semi-disabled in > > commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28 > Author: Mario Kleiner > Date: Tue May 13 00:42:08 2014 +0200 > > drm/nouveau/kms/nv04-nv40: fix pageflip events via special case. > > Unfortunately that hack got uprooted in > > commit cc1ef118fc099295ae6aabbacc8af94d8d8885eb > Author: Thierry Reding > Date: Wed Aug 12 17:00:31 2015 +0200 > > drm/irq: Make pipe unsigned and name consistent > > Trigering a warning when trying to sample the vblank timestamp for a > non-existing pipe. There's a few ways to fix this: > > - Open-code the old behaviour, which just enshrines this slight > breakage of the userspace ABI. > > - Revert Mario's commit and again inflict broken timestamps, again not > pretty. > > - Fix this for real by delaying the pageflip TS until the next vblank > interrupt, thereby making it accurate. > > This patch implements the third option. Since having a page flip > interrupt that happens when the pageflip gets armed and not when it > completes in the next vblank seems to be fairly common (older i915 hw > works very similarly) create a new helper to arm vblank events for > such drivers. What happens when the page flip interrupt arrives during a vertical blank period? Presumably the userspace event will be deferred until the next vertical blank period, but the flip might already take effect in the current one. > +/** > + * drm_arm_vblank_event - arm vblanke event after pageflip > + * @crtc: the source CRTC of the vblank event Ye olde vblanke event? ;) (typo, same in the previous comment) The function name in this comment doesn't match the name of the function it describes. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [Mesa-dev] [RFC PATCH 5/8] nv50: prevent NULL pointer dereference with pipe_query functions
On 23.06.2015 06:02, Samuel Pitoiset wrote: > > > On 06/22/2015 10:52 PM, Ilia Mirkin wrote: >> If query_create fails, why would any of these functions get called? > > Because the HUD doesn't check if query_create() fails and it calls other > pipe_query functions with NULL pointer instead of a valid query object. Could the HUD code be fixed instead? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 1/9] dri2: Fix can_exchange() to allow page-flipping on new servers.
On Mon, 2012-02-20 at 05:59 +0100, Mario Kleiner wrote: > On 02/16/2012 11:04 AM, Michel Dänzer wrote: > > On Don, 2012-02-16 at 00:45 +0100, Mario Kleiner wrote: > >> can_exchange() fails on at least Xorg 1.12+. This fixes > >> it in the same way it was fixed in the ati& intel ddx. > >> > >> Signed-off-by: Mario Kleiner > >> --- > >> src/nouveau_dri2.c |2 +- > >> 1 files changed, 1 insertions(+), 1 deletions(-) > >> > >> diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c > >> index 3aa5ec5..5b62425 100644 > >> --- a/src/nouveau_dri2.c > >> +++ b/src/nouveau_dri2.c > >> @@ -160,7 +160,7 @@ can_exchange(DrawablePtr draw, PixmapPtr dst_pix, > >> PixmapPtr src_pix) > >>return ((DRI2CanFlip(draw)&& pNv->has_pageflip))&& > >>dst_pix->drawable.width == src_pix->drawable.width&& > >>dst_pix->drawable.height == src_pix->drawable.height&& > >> - dst_pix->drawable.depth == src_pix->drawable.depth&& > >> + dst_pix->drawable.bitsPerPixel == > >> src_pix->drawable.bitsPerPixel&& > >>dst_pix->devKind == src_pix->devKind; > >> } > >> > > > > Actually, it seems like the pixmap depths really should match, otherwise > > one could end up with the front pixmap depth not matching the window > > depth. Not sure that's a real problem right now, but it seems wonky at > > least... > > > > Have you investigated why the depths don't match? > > Depends on the meaning of "investigated": One of the pixmaps has depth > 24 bits (the pixmap of the root window) the other 32 bits (as requested > from the client via DRI2GetBuffersWithFormat for RGBA8 visuals). Both > have 32 bpp. I checked what the intel and ati ddx do. The ati ddx always > checked for matching drawable.bitsPerPixel since kms pageflip support > was implemented. The intel ddx does the same, but the code and comments > suggests they tried both and checking for matching depths probably > didn't work: > > cd xorg/drivers/xf86-video-intel/ > git log -p e2615cdeef078dbd2e834b68c437f098a92b941d > > So everybody does it like this currently, and it seems to work. Right. I must have been incorrectly thinking of flipping as changing the window pixmap. I still have some doubts — e.g. why is an RGBA visual chosen for a fullscreen window, and does this really have anything to do with changes in xserver 1.12, or rather in Mesa — but I think the change itself is okay. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 1/9] dri2: Fix can_exchange() to allow page-flipping on new servers.
On Don, 2012-02-16 at 00:45 +0100, Mario Kleiner wrote: > can_exchange() fails on at least Xorg 1.12+. This fixes > it in the same way it was fixed in the ati & intel ddx. > > Signed-off-by: Mario Kleiner > --- > src/nouveau_dri2.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c > index 3aa5ec5..5b62425 100644 > --- a/src/nouveau_dri2.c > +++ b/src/nouveau_dri2.c > @@ -160,7 +160,7 @@ can_exchange(DrawablePtr draw, PixmapPtr dst_pix, > PixmapPtr src_pix) > return ((DRI2CanFlip(draw) && pNv->has_pageflip)) && > dst_pix->drawable.width == src_pix->drawable.width && > dst_pix->drawable.height == src_pix->drawable.height && > - dst_pix->drawable.depth == src_pix->drawable.depth && > + dst_pix->drawable.bitsPerPixel == > src_pix->drawable.bitsPerPixel && > dst_pix->devKind == src_pix->devKind; > } > Actually, it seems like the pixmap depths really should match, otherwise one could end up with the front pixmap depth not matching the window depth. Not sure that's a real problem right now, but it seems wonky at least... Have you investigated why the depths don't match? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 9/9] dri2: Fix corner case crash for swaplimit > 1
On Don, 2012-02-16 at 00:45 +0100, Mario Kleiner wrote: > If a swaplimit > 1 is set on a server which > supports the swaplimit api (XOrg 1.12.0+), > the following can happen: > > 1. Client calls glXSwapBuffersMscOML() with a >swap target > 1 vblank in the future, or a >client calls glXSwapbuffers() while the swap >interval is set to > 1 (unusual but possible). > > 2. nouveau_dri2_finish_swap() is therefore called >only at the target vblank, instead of immediately. > > 3. Because of the deferred execution of >nouveu_dri2_finish_swap(), the OpenGL client >can call x-servers DRI2GetBuffersWithFormat() >before nouveau_dri2_finish_swap() executes and >it deletes pixmaps that would be needed by >nouveau_dri2_finish_swap() --> Segfault --> Crash. Pixmaps are reference counted, so it should be possible to fix this via proper reference counting. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] When will nouveau kernel tree be merged into master
On Thu, 2009-07-09 at 21:51 +0200, Eric Valette wrote: > Pekka Paalanen wrote: > > Sorry for causing that much trouble. > > > > On Thu, 09 Jul 2009 19:20:54 +0200 > > Eric Valette wrote: > > > >> Well I would suggest you to look at GALLIUM3D because using gi to clone > >> mesa master is not even working and even if you get a git snapshot it > >> does not build. > > > > http://nouveau.freedesktop.org/wiki/GalliumHowto > > Is there something unclear here? > > The command > git clone git://anongit.freedesktop.org/git/mesa/mesa > > Does not work for me while others do. It hangs indefinitely (I let it > run once half a day, doing strace to see that I was waiting to a read on > a socket). Maybe there's a firewall between you and anongit.freedesktop.org which is filtering the Git port (9418/tcp). -- Earthling Michel Dänzer |http://www.vmware.com Libre software enthusiast | Debian, X and DRI developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] Why was old TTM removed from drm.git?
On Wed, 2009-06-24 at 19:26 +0200, Thomas Hellström wrote: > > Just prior to the commit I sent out a message explaining what I was > going to do and why, but apparently it didn't make it to the list > (which seems to be the case of quite a few mails these days). What was the From: address and subject of that mail (or any others that were apparently lost)? I can't seem to find anything in the dri-devel moderation queue mails around the weekend, so apparently it was dropped before it reached mailman. Maybe some sf.net spam filter or something. -- Earthling Michel Dänzer |http://www.vmware.com Libre software enthusiast | Debian, X and DRI developer ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
