Re: Shared atomic state causing Weston repaint failure
Hello Daniel² et al, I apologies in advance if the things I bring up are a bit orthogonal to the current discussion. And I'm writing this from the view of VR, but I have not written a VR compositor so take my comments with a bit of salt. And also changing modes while using a VR headset is probably going to be ultra rare so not sure how much effort it is worth. So from a VR compositor getting blocked like this is a no-go as the user would quickly throw EPUKE. The situation is compounded by the fact that the VR compositor has no idea what the display compositor is doing with regards to setting modes so can not do any mitigating on its side (like displaying a black screen). Some solutions that springs to mind (some I admit are probably not possible). - Make sure we don't get into this situation by locking the resources of the VR crtc group or allocating enough bandwidth for the display compositor crtcs up front. - Add priority and preemption to atomic so that VR compositor can never be blocked. - Add X/Wayland protocol for the compositor to tell the VR compositor that a modeset might effect it, so it can display a black-screen during that time. - Make it possible for the VR compositor to tell the kernel what it should do this case, like show black if I happen to get block before I can queue a new pageflip. Cheers, Jakob. On Wed, Jul 4, 2018 at 8:58 PM Daniel Vetter wrote: > > On Wed, Jul 4, 2018 at 5:44 PM, Daniel Stone wrote: > > Hi, > > The atomic API being super-explicit about how userspace sequences its > > calls is great and all, but having shared global state implicitly > > dragged in is kind of ruining my day. > > > > Currently on Intel, Weston sometimes fails on hotplug, because a > > commit which only enables CRTC B (not touching CRTC A or any other > > CRTC!), causes all commits to CRTC A to fail until CRTC B's modeset > > commit has fully retired: > > https://gitlab.freedesktop.org/wayland/weston/issues/24 > > > > The reason is that committing CRTC B resizes the DDB allocation for > > CRTC A as well, pulling CRTC A's CRTC state into the commit. This > > makes sense, but on the other hand it's totally opaque to userspace, > > and impossible for us to reason about when making commits. > > > > I suggested some options in that GitLab commit, none of which I like: > > * if any other CRTCs are pulled into a commit state, always execute > > a blocking commit in the kernel > > * if we're passing ALLOW_MODESET in userspace, only ever do blocking > > commits > > * whenever we get -EBUSY in userspace, assume we've been screwed by > > the kernel and defer until other outputs have completed > > * whenever we want to reconfigure any output in userspace, wait > > until all outputs are completely quiescent and do a single atomic > > commit covering all outputs > > > > The first one seems completely non-obvious from the kernel, but on the > > other hand the current -EBUSY failing behaviour is also non-obvious. > > > > The second is maybe the most reasonable, but on the other hand just > > working around a painful leaky abstraction: we also can't know upfront > > from userspace if this is actually going to be required, or if we're > > just killing responsiveness blocking for no reason. > > > > The third is the thing I least want to do, because it might well paper > > over legitimate bugs in userspace, and complicates our state tracking > > for no reason. > > > > The fourth is probably the most legitimate, but, well ... someone has > > to type up all the code to make our output-configuration API > > completely asynchronous. > > > > I suspect we're the first ones to be hitting this, because Weston has > > a truly independent per-CRTC repaint loop, we're one of the few atomic > > users, and also because Pekka did some seriously brutal hotplug > > testing whilst reworking Weston's output configuration API. Also > > because our approach to failed output repaints is to just freeze the > > output until it next cycles off and on, which is much more apparent > > than just silently dropping a frame here and there. ;) > > > > Any bright ideas on what could practically be done here? > > > > Cheers, > > Daniel > > We had an entirely inconclusive chat on irc about this topic. Random notes: > > - This can fail both on the commit doing a modeset (when it adds some > random other CRTC which still has a pending flip), and for normal > flips (if they run into a CRTC which has been affected by a modeset). > > - Just waiting for all nonblocking modesets to complete is not enough, > because you don't get an event for these affected CRTC. And they might > take longer. So you can still get an EBUSY even when it looks like > everything is done. > > - If you don't combine nonblocking with ALLOW_MODESET there's no > issue, because everyone just blocks (and maybe misses a few frames) > until the modeset is done. > > - We want to tell apart an EBUSY due to the redraw loop having lost > sync from an EBUSY due to these mode
[PATCH] drm/vmwgfx: fix lock breakage
On Thu, Oct 30, 2014 at 6:40 PM, Rob Clark wrote: > On Thu, Oct 30, 2014 at 1:39 PM, Rob Clark wrote: >> After: >> >> commit d059f652e73c35678d28d4cd09ab2cec89696af9 >> Author: Daniel Vetter >> AuthorDate: Fri Jul 25 18:07:40 2014 +0200 >> >> drm: Handle legacy per-crtc locking with full acquire ctx >> >> drm_mode_cursor_common() was switched to use drm_modeset_(un)lock_crtc() >> which uses full aquire ctx. So dropping/reaquiring the lock via >> drm_modeset_(un)lock() directly isn't the right thing to do, as lockdep >> kindly points out. >> >> The 'FIXME's about sorting out whether vmwgfx *really* needs to lock-all >> for cursor updates still apply. >> > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1155825 > >> Signed-off-by: Rob Clark >> --- >> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 8 >> 1 file changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Jakob Bornecrantz >> >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> index d2bc2b0..8fc1e38 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> @@ -187,7 +187,7 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc, struct >> drm_file *file_priv, >> * can do this since the caller in the drm core doesn't check >> anything >> * which is protected by any looks. >> */ >> - drm_modeset_unlock(&crtc->mutex); >> + drm_modeset_unlock_crtc(crtc); >> drm_modeset_lock_all(dev_priv->dev); >> >> /* A lot of the code assumes this */ >> @@ -252,7 +252,7 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc, struct >> drm_file *file_priv, >> ret = 0; >> out: >> drm_modeset_unlock_all(dev_priv->dev); >> - drm_modeset_lock(&crtc->mutex, NULL); >> + drm_modeset_lock_crtc(crtc); >> >> return ret; >> } >> @@ -273,7 +273,7 @@ int vmw_du_crtc_cursor_move(struct drm_crtc *crtc, int >> x, int y) >> * can do this since the caller in the drm core doesn't check >> anything >> * which is protected by any looks. >> */ >> - drm_modeset_unlock(&crtc->mutex); >> + drm_modeset_unlock_crtc(crtc); >> drm_modeset_lock_all(dev_priv->dev); >> >> vmw_cursor_update_position(dev_priv, shown, >> @@ -281,7 +281,7 @@ int vmw_du_crtc_cursor_move(struct drm_crtc *crtc, int >> x, int y) >>du->cursor_y + du->hotspot_y); >> >> drm_modeset_unlock_all(dev_priv->dev); >> - drm_modeset_lock(&crtc->mutex, NULL); >> + drm_modeset_lock_crtc(crtc); >> >> return 0; >> } >> -- >> 1.9.3 >> > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 00/18] Final batch of Android related fixes
On Sun, Sep 7, 2014 at 11:29 PM, Emil Velikov wrote: > Hello list, > > Here is the final batch that I've been planning to get upstreamed. > Everything else that remains are some custom downstream "hacks" that > will get upstreamed by their original authors in due time :) > > > Highlights: > - Drop a few unneeded Makefiles. > - Android support for libkms & modetest. Inspired by Benjamin > Gaignard's work. > - Private mmap/munmap wrappers to hide all the love that bionic has for > us :) > > > The series is available in branch 'android-final-fixes' at > https://github.com/evelikov/libdrm > > > Any comments, reviews, it builds or it works are appreciated. While I'm not that familiar with the Android build system patches 01, 02 and 03 - 18 are Reviewed-by: Jakob Bornecrantz You will have to ask the intel people about 810 and 830 headers removal. Cheers, Jakob.
[PATCH 05/18] libkms: build the intel backend only when needed
On Sun, Sep 7, 2014 at 11:30 PM, Emil Velikov wrote: > Signed-off-by: Emil Velikov > --- > configure.ac | 3 +++ > libkms/Makefile.am | 5 - > libkms/linux.c | 16 +++- > 3 files changed, 18 insertions(+), 6 deletions(-) Reviewed-by: Jakob Bornecrantz Cheers, Jakob. > > diff --git a/configure.ac b/configure.ac > index 484084f..f1d3451 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -270,6 +270,9 @@ fi > AM_CONDITIONAL(HAVE_LIBKMS, [test "x$LIBKMS" = xyes]) > > AM_CONDITIONAL(HAVE_INTEL, [test "x$INTEL" = xyes]) > +if test "x$INTEL" = xyes; then > + AC_DEFINE(HAVE_INTEL, 1, [Have intel support]) > +fi > > AM_CONDITIONAL(HAVE_VMWGFX, [test "x$VMWGFX" = xyes]) > if test "x$VMWGFX" = xyes; then > diff --git a/libkms/Makefile.am b/libkms/Makefile.am > index 449a73b..dae44e9 100644 > --- a/libkms/Makefile.am > +++ b/libkms/Makefile.am > @@ -15,7 +15,6 @@ libkms_la_LIBADD = ../libdrm.la > libkms_la_SOURCES = \ > internal.h \ > linux.c \ > - intel.c \ > dumb.c \ > api.c > > @@ -23,6 +22,10 @@ if HAVE_VMWGFX > libkms_la_SOURCES += vmwgfx.c > endif > > +if HAVE_INTEL > +libkms_la_SOURCES += intel.c > +endif > + > if HAVE_NOUVEAU > libkms_la_SOURCES += nouveau.c > endif > diff --git a/libkms/linux.c b/libkms/linux.c > index 17e1d58..77a0bbe 100644 > --- a/libkms/linux.c > +++ b/libkms/linux.c > @@ -103,25 +103,31 @@ linux_from_sysfs(int fd, struct kms_driver **out) > if (ret) > return ret; > > +#ifdef HAVE_INTEL > if (!strcmp(name, "intel")) > ret = intel_create(fd, out); > + else > +#endif > #ifdef HAVE_VMWGFX > - else if (!strcmp(name, "vmwgfx")) > + if (!strcmp(name, "vmwgfx")) > ret = vmwgfx_create(fd, out); > + else > #endif > #ifdef HAVE_NOUVEAU > - else if (!strcmp(name, "nouveau")) > + if (!strcmp(name, "nouveau")) > ret = nouveau_create(fd, out); > + else > #endif > #ifdef HAVE_RADEON > - else if (!strcmp(name, "radeon")) > + if (!strcmp(name, "radeon")) > ret = radeon_create(fd, out); > + else > #endif > #ifdef HAVE_EXYNOS > - else if (!strcmp(name, "exynos")) > + if (!strcmp(name, "exynos")) > ret = exynos_create(fd, out); > -#endif > else > +#endif > ret = -ENOSYS; > > free(name); > -- > 2.0.2 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/18] libkms: add Android build
On Sun, Sep 7, 2014 at 11:30 PM, Emil Velikov wrote: > Cc: Benjamin Gaignard > Signed-off-by: Emil Velikov > --- > Android.mk| 3 ++- > libkms/Android.mk | 53 + > 2 files changed, 55 insertions(+), 1 deletion(-) > create mode 100644 libkms/Android.mk Not super familiar with Android build system but it looks good to me: Reviewed-by: Jakob Bornecrantz Cheers, Jakob. > > diff --git a/Android.mk b/Android.mk > index 97a7d75..4d02b05 100644 > --- a/Android.mk > +++ b/Android.mk > @@ -55,7 +55,8 @@ SUBDIRS := \ > freedreno \ > intel \ > nouveau \ > - radeon > + radeon \ > + libkms > > mkfiles := $(patsubst %,$(LIBDRM_TOP)/%/Android.mk,$(SUBDIRS)) > include $(mkfiles) > diff --git a/libkms/Android.mk b/libkms/Android.mk > new file mode 100644 > index 000..d2df32a > --- /dev/null > +++ b/libkms/Android.mk > @@ -0,0 +1,53 @@ > +DRM_GPU_DRIVERS := $(strip $(filter-out swrast, $(BOARD_GPU_DRIVERS))) > + > +intel_drivers := i915 i965 i915g ilo > +radeon_drivers := r300g r600g radeonsi > +nouveau_drivers := nouveau > +vmwgfx_drivers := vmwgfx > + > +valid_drivers := \ > + $(intel_drivers) \ > + $(radeon_drivers) \ > + $(nouveau_drivers) \ > + $(vmwgfx_drivers) > + > +# warn about invalid drivers > +invalid_drivers := $(filter-out $(valid_drivers), $(DRM_GPU_DRIVERS)) > +ifneq ($(invalid_drivers),) > +$(warning invalid GPU drivers: $(invalid_drivers)) > +# tidy up > +DRM_GPU_DRIVERS := $(filter-out $(invalid_drivers), $(DRM_GPU_DRIVERS)) > +endif > + > +LOCAL_PATH := $(call my-dir) > + > +include $(CLEAR_VARS) > +include $(LOCAL_PATH)/Makefile.sources > + > +LOCAL_SRC_FILES := $(LIBKMS_FILES) > + > +ifneq ($(filter $(vmwgfx_drivers), $(DRM_GPU_DRIVERS)),) > +LOCAL_SRC_FILES += $(LIBKMS_VMWGFX_FILES) > +endif > + > +ifneq ($(filter $(intel_drivers), $(DRM_GPU_DRIVERS)),) > +LOCAL_SRC_FILES += $(LIBKMS_INTEL_FILES) > +endif > + > +ifneq ($(filter $(nouveau_drivers), $(DRM_GPU_DRIVERS)),) > +LOCAL_SRC_FILES += $(LIBKMS_NOUVEAU_FILES) > +endif > + > +ifneq ($(filter $(radeon_drivers), $(DRM_GPU_DRIVERS)),) > +LOCAL_SRC_FILES += $(LIBKMS_RADEON_FILES) > +endif > + > +LOCAL_MODULE := libkms > +LOCAL_SHARED_LIBRARIES := libdrm > + > +LOCAL_C_INCLUDES += $(TARGET_OUT_HEADERS)/libdrm > + > +LOCAL_COPY_HEADERS_TO := libdrm > +LOCAL_COPY_HEADERS := $(LIBKMS_H_FILES) > + > +include $(BUILD_SHARED_LIBRARY) > -- > 2.0.2 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vmwgfx: Fix incorrect write to read-only register
- Original Message - > Commit "drm/vmwgfx: correct fb_fix_screeninfo.line_length", while fixing a > vmwgfx fbdev bug, also writes the pitch to a supposedly read-only register: > SVGA_REG_BYTES_PER_LINE, while it should be (and also in fact is) written to > SVGA_REG_PITCHLOCK. > > There has been some reports of incorrect rendering with the mentioned commit > and Ubuntu 12.04. While hard to reproduce, hopefully this patch will correct > that issue. > > Cc: stable at vger.kernel.org > Cc: Christopher Friedt > Tested-by: Christopher Friedt > Signed-off-by: Thomas Hellstrom > --- > drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 1 - > 1 file changed, 1 deletion(-) Reviewed-by: Jakob Bornecrantz > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c > index a89ad93..b031b48 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c > @@ -179,7 +179,6 @@ static int vmw_fb_set_par(struct fb_info *info) > vmw_write(vmw_priv, SVGA_REG_DISPLAY_POSITION_Y, > info->var.yoffset); > vmw_write(vmw_priv, SVGA_REG_DISPLAY_WIDTH, info->var.xres); > vmw_write(vmw_priv, SVGA_REG_DISPLAY_HEIGHT, info->var.yres); > - vmw_write(vmw_priv, SVGA_REG_BYTES_PER_LINE, > info->fix.line_length); > vmw_write(vmw_priv, SVGA_REG_DISPLAY_ID, SVGA_ID_INVALID); > } > > -- > 1.8.3.2 >
[PATCH] drm/ttm: Don't clear page metadata of imported sg pages
- Ursprungligt meddelande -
> These page pointers shouldn't be visible to TTM in the first place, but
> until we fix that up, don't clear the page metadata because that
> will upset the exporter.
>
> Reported-by: Cristoph Haag
> Signed-off-by: Thomas Hellstrom
> ---
> drivers/gpu/drm/ttm/ttm_tt.c |3 +++
> 1 file changed, 3 insertions(+)
Reviewed-by: Jakob Bornecrantz
>
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index 9af9908..75f3190 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -380,6 +380,9 @@ static void ttm_tt_clear_mapping(struct ttm_tt *ttm)
> pgoff_t i;
> struct page **page = ttm->pages;
>
> + if (ttm->page_flags & TTM_PAGE_FLAG_SG)
> + return;
> +
> for (i = 0; i < ttm->num_pages; ++i) {
> (*page)->mapping = NULL;
> (*page++)->index = 0;
> --
> 1.7.10.4
>
[patch] tests/kmstest: support exynos
On Tue, Jan 7, 2014 at 7:40 AM, Hyungwon Hwang wrote: > tests/kmstest: support exynos > > Add exynos to list of kmstest supported modules. > > Signed-off-by: Hyungwon Hwang > --- > libkms/internal.h|2 ++ > libkms/linux.c |4 > tests/kmstest/main.c |1 + > 3 files changed, 7 insertions(+) Reviewed-by: Jakob Bornecrantz Do you need somebody to push this to the drm repo? Cheers, Jakob.
[PATCH 82/85] drivers: gpu: Mark functions as static in vmwgfx_kms.c
On Mon, Jan 6, 2014 at 5:48 PM, Rashika Kheria wrote: > Mark functions as static because they are not used outside the file > drm/vmwgfx/vmwgfx_kms.c. > > This eliminates the following warnings in drm/vmwgfx/vmwgfx_kms.c: > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c:43:6: warning: no previous prototype for > ?vmw_clip_cliprects? [-Wmissing-prototypes] > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c:426:6: warning: no previous prototype for > ?vmw_framebuffer_surface_destroy? [-Wmissing-prototypes] > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c:592:5: warning: no previous prototype for > ?vmw_framebuffer_surface_dirty? [-Wmissing-prototypes] > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c:757:6: warning: no previous prototype for > ?vmw_framebuffer_dmabuf_destroy? [-Wmissing-prototypes] > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c:943:5: warning: no previous prototype for > ?vmw_framebuffer_dmabuf_dirty? [-Wmissing-prototypes] > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c:1666:5: warning: no previous prototype > for ?vmw_du_update_layout? [-Wmissing-prototypes] > > Signed-off-by: Rashika Kheria > Reviewed-by: Josh Triplett > --- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) This and patch 83, 84, 85 are Reviewed-by: Jakob Bornecrantz Cheers, Jakob.
[PATCH 00/50] more drm de-midlayering
The vmwgfx changes are: Reviewed-by: Jakob Bornecrantz On Wed, Dec 11, 2013 at 11:34 AM, Daniel Vetter wrote: > Hi all, > > This series almost removes drm_bus, the last thing remaining is the > ->setversion > callback. Unfortunately we can't kill that completely since we need the > backwards compat cruft for pci domain bonghits on alpha/ppc. > > I've also shot at a few easy marks on the road while at it. > > My plan is to tackle the setversion mess in 3.15 with a Gross Hack. And then > convert a few drivers to allocate struct drm_device themselves and so complete > the demidlayering in the driver load paths. For simplicity that'd probably be > udl or the arm drivers due to lack of legacy baggage. That should allow us to > ditch drm_usb.c and drm_platform.c from the tree. > > My real long-term goal is to eventually use devres.c and similar cool stuff to > clean up the setup/teardown hell we have in i915.ko. Hence also why I've > deprecated the legacy agp/ums support in i915 in the hope that I'll get to > deal > with less cruft. Also maybe we can sort out the lifetime issues around sysfs > and > debugfs at driver/module unload time eventually ... just let me dream ;-) > > Also a cleanup from Dan on top. All the little fixes for Wu Fengguang > reported are > squashed in - he's now also doing some arm builds! > > Dan Carpenter (1): > drm: use memdup_user() as a cleanup > > Daniel Vetter (49): > drm/rcar: call drm_put_dev directly in the ->remove hook > drm/exynos: call drm_put_dev directly from ->remove > drm/imx: directly call drm_put_dev in ->remove > drm/tilcdc: call drm_put_dev directly from ->remove > drm/omap: call drm_put_dev directly in ->remove > drm/shmob: call drm_put_dev directly from ->remove hook > drm/armada: directly call drm_put_dev in ->remove > drm/msm: call drm_put_dev directly in ->remove > drm: rip out drm_platform_exit > drm: restrict the device list for shadow attached drivers > drm/bufs: remove handling of _DRM_GEM mappings > drm: kill DRIVER_REQUIRE_AGP > drm: ->agp_init can't fail > drm: rip out drm_core_has_AGP > drm: remove agp_init() bus callback > drm: inline drm_agp_destroy > drm: kill the ->agp_destroy callback > drm: remove global_mutex locking around agp_init > drm: rip out DRM_AGP_MEM and DRM_AGP_KERN > drm: Kill DRM_HZ > drm: Kill DRM_IRQ_ARGS > drm: Kill DRM_WAKUP and DRM_INIT_WAITQUEUE > drm: Kill DRM_COPY_(TO|FROM)_USER > drm: Kill DRM_*MEMORYBARRIER > drm: Kill DRM_SUSER > drm/gma500: Remove dead code > drm/irq: Replace DRM_WAIT_ON with wait_event > drm: Remove DRM_WAIT_ON from all drivers > drm/irq: simplify irq checks in drm_wait_vblank > drm/pci: fold in irq_by_busid support > drm/irq: drm_control is a legacy ioctl, so pci devices only > drm/irq: remove cargo-culted locking from irq_install/unistall > drm: remove drm_dev_to_irq from drivers > drm: kill drm_bus->bus_type > drm: Rip out totally bogus vga_switcheroo->can_switch locking > drm: rename dev->count_lock to dev->buf_lock > drm/irq: track the irq installed in drm_irq_install in dev->irq > drm/irq: Look up the pci irq directly in the drm_control ioctl > drm: pass the irq explicitly to drm_irq_install > drm: remove bus->get_irq implementations > drm: inline drm_pci_set_unique > drm: rip out dev->devname > drm: remove drm_bus->get_name > drm: Remove dev->kdriver > drm/: don't set driver->dev_priv_size to 0 > drm: store the gem vma offset manager in a typed pointer > drm: rip out dev->ioctl_count tracking > drm: Kill file_priv->ioctl_count tracking > drm: remove dev->vma_count > > Documentation/DocBook/drm.tmpl | 10 +-- > drivers/gpu/drm/armada/armada_drv.c | 5 +- > drivers/gpu/drm/ast/ast_drv.c| 1 - > drivers/gpu/drm/cirrus/cirrus_drv.h | 2 +- > drivers/gpu/drm/drm_agpsupport.c | 28 ++- > drivers/gpu/drm/drm_buffer.c | 2 +- > drivers/gpu/drm/drm_bufs.c | 42 +- > drivers/gpu/drm/drm_drv.c| 4 - > drivers/gpu/drm/drm_fops.c | 11 +-- > drivers/gpu/drm/drm_gem.c| 27 +++ > drivers/gpu/drm/drm_info.c | 22 +++--- > drivers/gpu/drm/drm_ioctl.c | 13 +-- > drivers/gpu/drm/drm_irq.c| 125 +++-- > drivers/gpu/drm/drm_memory.c | 15 ++-- > drivers/gpu/drm/drm_pci.c| 132 > --- > drivers/gpu/drm/drm_platform.c | 37 - > d
[PATCH] drm/ttm: Fix accesses through vmas with only partial coverage
Looks good to me.
Cheers, Jakob.
- Thomas Hellstrom wrote:
> VMAs covering a bo but that didn't start at the same address space offset as
> the bo they were mapping were incorrectly generating SEGFAULT errors in
> the fault handler.
>
> Reported-by: Joseph Dolinak
> Signed-off-by: Thomas Hellstrom
> Cc: stable at vger.kernel.org
> ---
> drivers/gpu/drm/ttm/ttm_bo_vm.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index b249ab9..6440eea 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -169,9 +169,9 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma,
> struct vm_fault *vmf)
> }
>
> page_offset = ((address - vma->vm_start) >> PAGE_SHIFT) +
> - drm_vma_node_start(&bo->vma_node) - vma->vm_pgoff;
> - page_last = vma_pages(vma) +
> - drm_vma_node_start(&bo->vma_node) - vma->vm_pgoff;
> + vma->vm_pgoff - drm_vma_node_start(&bo->vma_node);
> + page_last = vma_pages(vma) + vma->vm_pgoff -
> + drm_vma_node_start(&bo->vma_node);
>
> if (unlikely(page_offset >= bo->num_pages)) {
> retval = VM_FAULT_SIGBUS;
> --
> 1.7.10.4
[PATCH] drm: Push dirtyfb ioctl kms locking down to drivers
Looks good to me
Reviewed-by: Jakob Bornecrant
On Wed, Dec 4, 2013 at 1:13 PM, wrote:
> From: Ville Syrj?l?
>
> Not all drivers will need take all the modeset locks for dirtyfb, so
> push the locking down to the drivers.
>
> Signed-off-by: Ville Syrj?l?
> Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_crtc.c | 2 --
> drivers/gpu/drm/omapdrm/omap_fb.c | 4
> drivers/gpu/drm/qxl/qxl_display.c | 9 -
> drivers/gpu/drm/udl/udl_fb.c| 12 +---
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 18 --
> 5 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index d6cf77c..266a01d 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2767,10 +2767,8 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
> }
>
> if (fb->funcs->dirty) {
> - drm_modeset_lock_all(dev);
> ret = fb->funcs->dirty(fb, file_priv, flags, r->color,
>clips, num_clips);
> - drm_modeset_unlock_all(dev);
> } else {
> ret = -ENOSYS;
> }
> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c
> b/drivers/gpu/drm/omapdrm/omap_fb.c
> index f2b8f06..f466c4a 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fb.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fb.c
> @@ -123,12 +123,16 @@ static int omap_framebuffer_dirty(struct
> drm_framebuffer *fb,
> {
> int i;
>
> + drm_modeset_lock_all(fb->dev);
> +
> for (i = 0; i < num_clips; i++) {
> omap_framebuffer_flush(fb, clips[i].x1, clips[i].y1,
> clips[i].x2 - clips[i].x1,
> clips[i].y2 - clips[i].y1);
> }
>
> + drm_modeset_unlock_all(fb->dev);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c
> b/drivers/gpu/drm/qxl/qxl_display.c
> index 5e827c2..b8f3bc7 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -399,10 +399,14 @@ static int qxl_framebuffer_surface_dirty(struct
> drm_framebuffer *fb,
> struct qxl_bo *qobj;
> int inc = 1;
>
> + drm_modeset_lock_all(fb->dev);
> +
> qobj = gem_to_qxl_bo(qxl_fb->obj);
> /* if we aren't primary surface ignore this */
> - if (!qobj->is_primary)
> + if (!qobj->is_primary) {
> + drm_modeset_unlock_all(fb->dev);
> return 0;
> + }
>
> if (!num_clips) {
> num_clips = 1;
> @@ -417,6 +421,9 @@ static int qxl_framebuffer_surface_dirty(struct
> drm_framebuffer *fb,
>
> qxl_draw_dirty_fb(qdev, qxl_fb, qobj, flags, color,
> clips, num_clips, inc);
> +
> + drm_modeset_unlock_all(fb->dev);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
> index 97e9d61..dbadd49 100644
> --- a/drivers/gpu/drm/udl/udl_fb.c
> +++ b/drivers/gpu/drm/udl/udl_fb.c
> @@ -403,15 +403,17 @@ static int udl_user_framebuffer_dirty(struct
> drm_framebuffer *fb,
> int i;
> int ret = 0;
>
> + drm_modeset_lock_all(fb->dev);
> +
> if (!ufb->active_16)
> - return 0;
> + goto unlock;
>
> if (ufb->obj->base.import_attach) {
> ret =
> dma_buf_begin_cpu_access(ufb->obj->base.import_attach->dmabuf,
>0, ufb->obj->base.size,
>DMA_FROM_DEVICE);
> if (ret)
> - return ret;
> + goto unlock;
> }
>
> for (i = 0; i < num_clips; i++) {
> @@ -419,7 +421,7 @@ static int udl_user_framebuffer_dirty(struct
> drm_framebuffer *fb,
> clips[i].x2 - clips[i].x1,
> clips[i].y2 - clips[i].y1);
> if (ret)
> - break;
> + goto unlock;
> }
>
> if (ufb->obj->base.import_attach) {
> @@ -427,6 +429,10 @@ static int udl_user_framebuffer_dirty(struct
> drm_framebuffer *fb,
>0, ufb->obj->base.size,
>DMA_FROM_DEVICE);
> }
> +
> + unlock:
> + drm_modeset_unlock_all(fb->dev);
> +
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index ecb3d86..ab0b88f 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -608,9 +608,13 @@ int vmw_framebuffer_surface_dirty(struct drm_framebuffer
> *framebuffer,
> if (!dev_priv->sou_priv)
> return -EINVAL;
>
> + drm_modeset_lock_all(dev_priv->dev);
> +
> ret = ttm_read_lock(&vmaster->lock, true);
> -
[PATCH] drm/ttm: Remove set_need_resched from the ttm fault handler
On Thu, Nov 14, 2013 at 7:49 PM, Thomas Hellstrom
wrote:
> Addresses
> "[BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE".
>
> In the first occurence it was used to try to be nice while releasing the
> mmap_sem and retrying the fault to work around a locking inversion.
> The second occurence was never used.
>
> There has been some discussion whether we should change the locking order to
> mmap_sem -> bo_reserve. This patch doesn't address that issue, and leaves
> that locking order undefined. The solution that we release the mmap_sem if
> tryreserve fails and wait for the buffer to become unreserved is something
> we want in any case, and follows how the core vm system waits for pages
> to be come unlocked while releasing the mmap_sem.
>
> The code also outlines what needs to be changed if we want to establish the
> locking order as mmap_sem -> bo::reserve.
>
> One slight issue that remains with this code is that the fault handler might
> be prone to starvation if another thread countinously reserves the buffer.
> IMO that usage pattern is highly unlikely.
>
> Signed-off-by: Thomas Hellstrom
Reviewed-by: Jakob Bornecrantz
> ---
> drivers/gpu/drm/ttm/ttm_bo.c| 35 ++-
> drivers/gpu/drm/ttm/ttm_bo_vm.c | 26 --
> include/drm/ttm/ttm_bo_api.h|4 +++-
> 3 files changed, 57 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 8d5a646..07e02c4 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -151,7 +151,7 @@ static void ttm_bo_release_list(struct kref *list_kref)
> atomic_dec(&bo->glob->bo_count);
> if (bo->resv == &bo->ttm_resv)
> reservation_object_fini(&bo->ttm_resv);
> -
> + mutex_destroy(&bo->wu_mutex);
> if (bo->destroy)
> bo->destroy(bo);
> else {
> @@ -1123,6 +1123,7 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
> INIT_LIST_HEAD(&bo->ddestroy);
> INIT_LIST_HEAD(&bo->swap);
> INIT_LIST_HEAD(&bo->io_reserve_lru);
> + mutex_init(&bo->wu_mutex);
> bo->bdev = bdev;
> bo->glob = bdev->glob;
> bo->type = type;
> @@ -1704,3 +1705,35 @@ void ttm_bo_swapout_all(struct ttm_bo_device *bdev)
> ;
> }
> EXPORT_SYMBOL(ttm_bo_swapout_all);
> +
> +/**
> + * ttm_bo_wait_unreserved - interruptible wait for a buffer object to become
> + * unreserved
> + *
> + * @bo: Pointer to buffer
> + */
> +int ttm_bo_wait_unreserved(struct ttm_buffer_object *bo)
> +{
> + int ret;
> +
> + /*
> +* In the absense of a wait_unlocked API,
> +* Use the bo::wu_mutex to avoid triggering livelocks due to
> +* concurrent use of this function. Note that this use of
> +* bo::wu_mutex can go away if we change locking order to
> +* mmap_sem -> bo::reserve.
> +*/
> + ret = mutex_lock_interruptible(&bo->wu_mutex);
> + if (unlikely(ret != 0))
> + return -ERESTARTSYS;
> + if (!ww_mutex_is_locked(&bo->resv->lock))
> + goto out_unlock;
> + ret = ttm_bo_reserve_nolru(bo, true, false, false, NULL);
> + if (unlikely(ret != 0))
> + goto out_unlock;
> + ww_mutex_unlock(&bo->resv->lock);
> +
> +out_unlock:
> + mutex_unlock(&bo->wu_mutex);
> + return ret;
> +}
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index ac617f3..b249ab9 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -107,13 +107,28 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma,
> struct vm_fault *vmf)
> /*
> * Work around locking order reversal in fault / nopfn
> * between mmap_sem and bo_reserve: Perform a trylock operation
> -* for reserve, and if it fails, retry the fault after scheduling.
> +* for reserve, and if it fails, retry the fault after waiting
> +* for the buffer to become unreserved.
> */
> -
> - ret = ttm_bo_reserve(bo, true, true, false, 0);
> + ret = ttm_bo_reserve(bo, true, true, false, NULL);
> if (unlikely(ret != 0)) {
> - if (ret == -EBUSY)
> - set_need_resched();
> + if (ret != -EBUSY)
> + return VM_FAULT_NOPAGE;
> +
> + if (vmf->flags & FAULT_FLAG_ALLOW_RETRY) {
>
[PATCH] drm/ttm: Don't move non-existing data
- Original Message -
> If ttm_bo_move_memcpy was instructed to move a non-populated ttm to
> io memory, it would first populate the ttm, then move the data and then
> destroy the ttm. That's stupid. However, some drivers might have relied on
> this to clear io memory from old stuff. So instead of a NOP, which would
> be the most efficient, just clear the destination.
>
> Signed-off-by: Thomas Hellstrom
Reviewed-by: Jakob Bornecrantz
Cheers, Jakob.
> ---
> drivers/gpu/drm/ttm/ttm_bo_util.c |7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
> b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 4834c46..15b86a9 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -350,10 +350,13 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
> goto out2;
>
> /*
> - * Move nonexistent data. NOP.
> + * Don't move nonexistent data. Clear destination instead.
>*/
> - if (old_iomap == NULL && ttm == NULL)
> + if (old_iomap == NULL &&
> + (ttm == NULL || ttm->state == tt_unpopulated)) {
> + memset_io(new_iomap, 0, new_mem->num_pages*PAGE_SIZE);
> goto out2;
> + }
>
> /*
>* TTM might be null for moves within the same region.
> --
> 1.7.10.4
>
dma-buf non-coherent mmap
On Fri, Nov 1, 2013 at 1:25 AM, Rob Clark wrote: > On Thu, Oct 31, 2013 at 8:17 PM, Jakob Bornecrantz > wrote: >> On Fri, Nov 1, 2013 at 12:00 AM, Daniel Vetter wrote: >>> On Thu, Oct 31, 2013 at 10:07:25PM +0100, Thomas Hellstrom wrote: >>>> On 10/31/2013 09:48 PM, Dave Airlie wrote: >>>> >On Fri, Nov 1, 2013 at 6:40 AM, Thomas Hellstrom >>> >vmware.com> wrote: >>>> >>Well, I'd be happy to avoid mmap, but then what does optional mean in >>>> >>this >>>> >>context? >>>> >>That all generic user-space apps *must* implement a workaround if mmap >>>> >>isn't >>>> >>implemented? >>>> >> >>>> >>It's unfortunate a bit like implicit synchronization mentioned in >>>> >>section 3) >>>> >>in Direct Userspace Access/mmap Support >>>> >>in the kernel dma-buf doc: It should be avoided, otherwise it might be >>>> >>relied upon by userspace and exporters >>>> >>not implementing it will suffer. >>>> >> >>>> >>In reality, people will start using mmap() and won't care to implement >>>> >>workarounds if it's not supported, and drivers like >>>> >>vmwgfx and non-coherent architectures will suffer. >>>> >> >>>> >>I haven't looked closely at how DRI3 or Wayland/weston use or will use >>>> >>dma-buf, but if they rely on mmap, we're sort >>>> >>of lost. MIR uses the following scheme: >>>> >DRI3 and wayland won't use dma-buf mmap directly, >>>> > >>>> >using dma-buf mmap directly is wrong for anything that shares objects >>>> >with itself. >>>> >>>> That sounds good to hear. Perhaps we should add that to the dma-buf docs. >>> >>> Userspace mmap was essentially added as a concession to the android ion >>> guys since they really, really wanted it. We've tried to tell them that >>> it's a horrible idea (see all the fun with coherency and syncing), but >>> they said that they have userspace for it already and so we let it be. >>> >>> Imo if you're not running Android userspace there's no need for this at >>> all. >> >> But now it turns out that gstreamer is using it and our life is hell. We >> should >> have made it not work for _any_ driver if CONFIG_ANDRIOD wasn't set. > > well, at the moment mmap is only implemented by a few of the arm > drivers, from the looks of it. So I guess in the near term it is > mostly going to be of interest to the SoC crowd. Ah okay, I thought this was desktop gstreamer. > Not sure about the long term, perhaps we should see about > turning gallium pipe_transfer/map stuff into a kernel interface > (ioctl's directly on the dmabuf fd, perhaps?) Or they could just create a OpenGL context, I know it sounds heavy weight. But somebody will eventually want to synchronize this with a different client API in a non-blocking way. And OpenGL or some other Khronos API already have that integration, no real need to reinvent the wheel. Then again I think both GBM and DRI have hooks for mapping and unmapping buffers, so those might suffice. Cheers, Jakob.
dma-buf non-coherent mmap
On Fri, Nov 1, 2013 at 12:00 AM, Daniel Vetter wrote: > On Thu, Oct 31, 2013 at 10:07:25PM +0100, Thomas Hellstrom wrote: >> On 10/31/2013 09:48 PM, Dave Airlie wrote: >> >On Fri, Nov 1, 2013 at 6:40 AM, Thomas Hellstrom >> >wrote: >> >>Well, I'd be happy to avoid mmap, but then what does optional mean in this >> >>context? >> >>That all generic user-space apps *must* implement a workaround if mmap >> >>isn't >> >>implemented? >> >> >> >>It's unfortunate a bit like implicit synchronization mentioned in section >> >>3) >> >>in Direct Userspace Access/mmap Support >> >>in the kernel dma-buf doc: It should be avoided, otherwise it might be >> >>relied upon by userspace and exporters >> >>not implementing it will suffer. >> >> >> >>In reality, people will start using mmap() and won't care to implement >> >>workarounds if it's not supported, and drivers like >> >>vmwgfx and non-coherent architectures will suffer. >> >> >> >>I haven't looked closely at how DRI3 or Wayland/weston use or will use >> >>dma-buf, but if they rely on mmap, we're sort >> >>of lost. MIR uses the following scheme: >> >DRI3 and wayland won't use dma-buf mmap directly, >> > >> >using dma-buf mmap directly is wrong for anything that shares objects >> >with itself. >> >> That sounds good to hear. Perhaps we should add that to the dma-buf docs. > > Userspace mmap was essentially added as a concession to the android ion > guys since they really, really wanted it. We've tried to tell them that > it's a horrible idea (see all the fun with coherency and syncing), but > they said that they have userspace for it already and so we let it be. > > Imo if you're not running Android userspace there's no need for this at > all. But now it turns out that gstreamer is using it and our life is hell. We should have made it not work for _any_ driver if CONFIG_ANDRIOD wasn't set. Cheers, Jakob.
[PATCH] drm/ttm: Make NO_EVICT bos available to shrinkers pending destruction
On Thu, Oct 10, 2013 at 8:22 PM, Thomas Hellstrom
wrote:
> NO_EVICT bos that are not idle when all references are dropped are put on
> the delayed destroy list. However, since they are not on LRU lists, they
> are not available to shrinkers at that point, and buffers on the delayed
> destroy list are not checked very often for idle.
>
> So when these buffers are put on the delayed destroy list, clear the
> NO_EVICT flag and put them on the right LRU list. This way they are
> immediately available for eviction or shrinkers and will not cause false
> OOMS.
>
> Signed-off-by: Thomas Hellstrom
Reviewed-by: Jakob Bornecrantz
> ---
> drivers/gpu/drm/ttm/ttm_bo.c | 14 +-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index f1a857e..6c1a38f 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -429,8 +429,20 @@ static void ttm_bo_cleanup_refs_or_queue(struct
> ttm_buffer_object *bo)
> sync_obj = driver->sync_obj_ref(bo->sync_obj);
> spin_unlock(&bdev->fence_lock);
>
> - if (!ret)
> + if (!ret) {
> +
> + /*
> +* Make NO_EVICT bos immediately available to
> +* shrinkers, now that they are queued for
> +* destruction.
> +*/
> + if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) {
> + bo->mem.placement &= ~TTM_PL_FLAG_NO_EVICT;
> + ttm_bo_add_to_lru(bo);
> + }
> +
> ww_mutex_unlock(&bo->resv->lock);
> + }
>
> kref_get(&bo->list_kref);
> list_add_tail(&bo->ddestroy, &bdev->ddestroy);
> --
> 1.7.10.4
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/ttm: Allow vm fault retries
On Thu, Oct 10, 2013 at 8:22 PM, Thomas Hellstrom
wrote:
> Make use of the FAULT_FLAG_ALLOW_RETRY flag to allow dropping the
> mmap_sem while waiting for bo idle.
>
> FAULT_FLAG_ALLOW_RETRY appears to be primarily designed for disk waits
> but should work just as fine for GPU waits..
>
> Signed-off-by: Thomas Hellstrom
Reviewed-by: Jakob Bornecrantz
Tho somebody else should also take a look at this.
> ---
> drivers/gpu/drm/ttm/ttm_bo_vm.c | 62
> +++
> 1 file changed, 50 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index 1006c15..c03514b 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -41,6 +41,51 @@
>
> #define TTM_BO_VM_NUM_PREFAULT 16
>
> +static int ttm_bo_vm_fault_idle(struct ttm_buffer_object *bo,
> + struct vm_area_struct *vma,
> + struct vm_fault *vmf)
> +{
> + struct ttm_bo_device *bdev = bo->bdev;
> + int ret = 0;
> +
> + spin_lock(&bdev->fence_lock);
> + if (likely(!test_bit(TTM_BO_PRIV_FLAG_MOVING, &bo->priv_flags)))
> + goto out_unlock;
> +
> + /*
> +* Quick non-stalling check for idle.
> +*/
> + ret = ttm_bo_wait(bo, false, false, true);
> + if (likely(ret == 0))
> + goto out_unlock;
> +
> + /*
> +* If possible, avoid waiting for GPU with mmap_sem
> +* held.
> +*/
> + if (vmf->flags & FAULT_FLAG_ALLOW_RETRY) {
> + ret = VM_FAULT_RETRY;
> + if (vmf->flags & FAULT_FLAG_RETRY_NOWAIT)
> + goto out_unlock;
> +
> + up_read(&vma->vm_mm->mmap_sem);
> + (void) ttm_bo_wait(bo, false, true, false);
> + goto out_unlock;
> + }
> +
> + /*
> +* Ordinary wait.
> +*/
> + ret = ttm_bo_wait(bo, false, true, false);
> + if (unlikely(ret != 0))
> + ret = (ret != -ERESTARTSYS) ? VM_FAULT_SIGBUS :
> + VM_FAULT_NOPAGE;
> +
> +out_unlock:
> + spin_unlock(&bdev->fence_lock);
> + return ret;
> +}
> +
> static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> {
> struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
> @@ -91,18 +136,11 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma,
> struct vm_fault *vmf)
> * Wait for buffer data in transit, due to a pipelined
> * move.
> */
> -
> - spin_lock(&bdev->fence_lock);
> - if (test_bit(TTM_BO_PRIV_FLAG_MOVING, &bo->priv_flags)) {
> - ret = ttm_bo_wait(bo, false, true, false);
> - spin_unlock(&bdev->fence_lock);
> - if (unlikely(ret != 0)) {
> - retval = (ret != -ERESTARTSYS) ?
> - VM_FAULT_SIGBUS : VM_FAULT_NOPAGE;
> - goto out_unlock;
> - }
> - } else
> - spin_unlock(&bdev->fence_lock);
> + ret = ttm_bo_vm_fault_idle(bo, vma, vmf);
> + if (unlikely(ret != 0)) {
> + retval = ret;
> + goto out_unlock;
> + }
>
> ret = ttm_mem_io_lock(man, true);
> if (unlikely(ret != 0)) {
> --
> 1.7.10.4
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
TTM Locking order of bo::reserve -> vm::mmap_sem
On Mon, Oct 21, 2013 at 10:48 AM, Thomas Hellstrom wrote: > Hi! > > As discussed previously the current locking order in TTM of these locks is > bo::reserve -> vm::mmap_sem. This leads to a hack in > the TTM fault() handle to try and revert the locking order. If a tryreserve > failed, we tried to have the vm code release the mmap_sem() and then > schedule, to give the holder of bo::reserve a chance to release the lock. > This solution is no longer legal, since we've been more or less kindly asked > to remove the set_need_resched() call. > > Maarten has proposed to invert the locking order. I've previously said I had > no strong preference. The current locking order dates back from the time > when TTM wasn't using unmap_mapping_range() but walked the page tables > itself, updating PTEs as needed. Furthermore it was needed for user bos that > used get_user_pages() in the TTM populate and swap-in methods. User-bos were > removed some time ago but I'm looking at re-adding them. They would suite > the VMware model of cached-only pages very well. I see uses both in the > gallium API, XA's DMA functionality and openCL. In particular OpenCL's clEnqueueWriteBuffer[1] function and friends which can move quite a bit of data in and out of VRAM. > > We would then need a somewhat nicer way to invert the locking order. I've > attached a solution that ups the mmap_sem and then reserves, but due to how > the fault API is done, we then need to release the reserve and retry the > fault. This of course opens up for starvation, but I don't think starvation > at this point is very likely: One thread being refused to write or read from > a buffer object because the GPU is continously busy with it. If this *would* > become a problem, it's probably possible to modify the fault code to allow > us to hold locks until the retried fault, but that would be a bit invasive, > since it touches the arch code > > Basically I'm proposing to keep the current locking order. Cheers, Jakob. [1] http://www.khronos.org/registry/cl/sdk/1.0/docs/man/xhtml/clEnqueueWriteBuffer.html
[PATCH] drm/ttm: Handle in memory region copies
Reviewed-by: Thomas Hellstr?m
Signed-off-by: Jakob Bornecrantz
Cc: Dave Airlie
Cc: dri-devel at lists.freedesktop.org
Cc: stable at vger.kernel.org
---
drivers/gpu/drm/ttm/ttm_bo_util.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 8be35c8..037f101 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -342,7 +342,9 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
if (old_iomap == NULL && ttm == NULL)
goto out2;
- if (ttm->state == tt_unpopulated) {
+ /* TTM might be null for moves within the same region.
+*/
+ if (ttm && ttm->state == tt_unpopulated) {
ret = ttm->bdev->driver->ttm_tt_populate(ttm);
if (ret) {
/* if we fail here don't nuke the mm node
--
1.8.1.2
[PATCH 0/7] drm: Return -ENOENT when objects can't be found
On Thu, Oct 17, 2013 at 12:34 PM, wrote: > We're rather inconsistent in which error values we return to userspace > on failure. I want to unify the behaviour a bit and consistently return > ENOENT when mode object lookups fail. We already do that in a few places > but in most places we just return EINVAL. > > I made a separate patch for each affected driver just in case there's some > magic meaning to the error values for certain drivers. > > > Ville Syrj?l? (7): > drm: Consistently return -ENOENT when a mode object can't be found > drm: Return -ENOENT when a framebuffer can't be found > drm/gma500: Return -ENOENT when a mode object can't be found > drm/i915: Return -ENOENT when a mode object can't be found > drm/radeon: Return -ENOENT when a mode object can't be found > drm/vmwgfx: Return -ENOENT when a mode object can't be found > drm/vmwgfx: Return -ENOENT when a framebuffer can't be found Sounds good to me, vmwgfx patches are: Reviewed-By: Jakob Bornecrantz Cheers, Jakob.
[PATCH] drm/vmwgfx: Split GMR2_REMAP commands if they are to large
This fixes the piglit test texturing/max-texture-size
causing the VM to die due to a too large SVGA command.
Signed-off-by: Jakob Bornecrantz
Reviewed-by: Biran Paul
Reviewed-by: Zack Rusin
---
drivers/gpu/drm/vmwgfx/vmwgfx_gmr.c | 58 +
1 file changed, 39 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gmr.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_gmr.c
index 3751730..1a0bf07 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gmr.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gmr.c
@@ -29,7 +29,9 @@
#include
#include
-#define VMW_PPN_SIZE sizeof(unsigned long)
+#define VMW_PPN_SIZE (sizeof(unsigned long))
+/* A future safe maximum remap size. */
+#define VMW_PPN_PER_REMAP ((31 * 1024) / VMW_PPN_SIZE)
static int vmw_gmr2_bind(struct vmw_private *dev_priv,
struct page *pages[],
@@ -38,43 +40,61 @@ static int vmw_gmr2_bind(struct vmw_private *dev_priv,
{
SVGAFifoCmdDefineGMR2 define_cmd;
SVGAFifoCmdRemapGMR2 remap_cmd;
- uint32_t define_size = sizeof(define_cmd) + 4;
- uint32_t remap_size = VMW_PPN_SIZE * num_pages + sizeof(remap_cmd) + 4;
uint32_t *cmd;
uint32_t *cmd_orig;
+ uint32_t define_size = sizeof(define_cmd) + sizeof(*cmd);
+ uint32_t remap_num = num_pages / VMW_PPN_PER_REMAP + ((num_pages %
VMW_PPN_PER_REMAP) > 0);
+ uint32_t remap_size = VMW_PPN_SIZE * num_pages + (sizeof(remap_cmd) +
sizeof(*cmd)) * remap_num;
+ uint32_t remap_pos = 0;
+ uint32_t cmd_size = define_size + remap_size;
uint32_t i;
- cmd_orig = cmd = vmw_fifo_reserve(dev_priv, define_size + remap_size);
+ cmd_orig = cmd = vmw_fifo_reserve(dev_priv, cmd_size);
if (unlikely(cmd == NULL))
return -ENOMEM;
define_cmd.gmrId = gmr_id;
define_cmd.numPages = num_pages;
+ *cmd++ = SVGA_CMD_DEFINE_GMR2;
+ memcpy(cmd, &define_cmd, sizeof(define_cmd));
+ cmd += sizeof(define_cmd) / sizeof(*cmd);
+
+ /*
+* Need to split the command if there are too many
+* pages that goes into the gmr.
+*/
+
remap_cmd.gmrId = gmr_id;
remap_cmd.flags = (VMW_PPN_SIZE > sizeof(*cmd)) ?
SVGA_REMAP_GMR2_PPN64 : SVGA_REMAP_GMR2_PPN32;
- remap_cmd.offsetPages = 0;
- remap_cmd.numPages = num_pages;
- *cmd++ = SVGA_CMD_DEFINE_GMR2;
- memcpy(cmd, &define_cmd, sizeof(define_cmd));
- cmd += sizeof(define_cmd) / sizeof(uint32);
+ while (num_pages > 0) {
+ unsigned long nr = min(num_pages, (unsigned
long)VMW_PPN_PER_REMAP);
+
+ remap_cmd.offsetPages = remap_pos;
+ remap_cmd.numPages = nr;
- *cmd++ = SVGA_CMD_REMAP_GMR2;
- memcpy(cmd, &remap_cmd, sizeof(remap_cmd));
- cmd += sizeof(remap_cmd) / sizeof(uint32);
+ *cmd++ = SVGA_CMD_REMAP_GMR2;
+ memcpy(cmd, &remap_cmd, sizeof(remap_cmd));
+ cmd += sizeof(remap_cmd) / sizeof(*cmd);
- for (i = 0; i < num_pages; ++i) {
- if (VMW_PPN_SIZE <= 4)
- *cmd = page_to_pfn(*pages++);
- else
- *((uint64_t *)cmd) = page_to_pfn(*pages++);
+ for (i = 0; i < nr; ++i) {
+ if (VMW_PPN_SIZE <= 4)
+ *cmd = page_to_pfn(*pages++);
+ else
+ *((uint64_t *)cmd) = page_to_pfn(*pages++);
- cmd += VMW_PPN_SIZE / sizeof(*cmd);
+ cmd += VMW_PPN_SIZE / sizeof(*cmd);
+ }
+
+ num_pages -= nr;
+ remap_pos += nr;
}
- vmw_fifo_commit(dev_priv, define_size + remap_size);
+ BUG_ON(cmd != cmd_orig + cmd_size / sizeof(*cmd));
+
+ vmw_fifo_commit(dev_priv, cmd_size);
return 0;
}
--
1.8.1.2
[PATCH] drm/vmwgfx: Split GMR2_REMAP commands if they are to large
This fixes the piglit test texturing/max-texture-size
causing the VM to die due to a too large SVGA command.
Signed-off-by: Jakob Bornecrantz
Reviewed-by: Biran Paul
Reviewed-by: Zack Rusin
---
drivers/gpu/drm/vmwgfx/vmwgfx_gmr.c | 58 +
1 file changed, 39 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gmr.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_gmr.c
index 3751730..1a0bf07 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gmr.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gmr.c
@@ -29,7 +29,9 @@
#include
#include
-#define VMW_PPN_SIZE sizeof(unsigned long)
+#define VMW_PPN_SIZE (sizeof(unsigned long))
+/* A future safe maximum remap size. */
+#define VMW_PPN_PER_REMAP ((31 * 1024) / VMW_PPN_SIZE)
static int vmw_gmr2_bind(struct vmw_private *dev_priv,
struct page *pages[],
@@ -38,43 +40,61 @@ static int vmw_gmr2_bind(struct vmw_private *dev_priv,
{
SVGAFifoCmdDefineGMR2 define_cmd;
SVGAFifoCmdRemapGMR2 remap_cmd;
- uint32_t define_size = sizeof(define_cmd) + 4;
- uint32_t remap_size = VMW_PPN_SIZE * num_pages + sizeof(remap_cmd) + 4;
uint32_t *cmd;
uint32_t *cmd_orig;
+ uint32_t define_size = sizeof(define_cmd) + sizeof(*cmd);
+ uint32_t remap_num = num_pages / VMW_PPN_PER_REMAP + ((num_pages %
VMW_PPN_PER_REMAP) > 0);
+ uint32_t remap_size = VMW_PPN_SIZE * num_pages + (sizeof(remap_cmd) +
sizeof(*cmd)) * remap_num;
+ uint32_t remap_pos = 0;
+ uint32_t cmd_size = define_size + remap_size;
uint32_t i;
- cmd_orig = cmd = vmw_fifo_reserve(dev_priv, define_size + remap_size);
+ cmd_orig = cmd = vmw_fifo_reserve(dev_priv, cmd_size);
if (unlikely(cmd == NULL))
return -ENOMEM;
define_cmd.gmrId = gmr_id;
define_cmd.numPages = num_pages;
+ *cmd++ = SVGA_CMD_DEFINE_GMR2;
+ memcpy(cmd, &define_cmd, sizeof(define_cmd));
+ cmd += sizeof(define_cmd) / sizeof(*cmd);
+
+ /*
+* Need to split the command if there are too many
+* pages that goes into the gmr.
+*/
+
remap_cmd.gmrId = gmr_id;
remap_cmd.flags = (VMW_PPN_SIZE > sizeof(*cmd)) ?
SVGA_REMAP_GMR2_PPN64 : SVGA_REMAP_GMR2_PPN32;
- remap_cmd.offsetPages = 0;
- remap_cmd.numPages = num_pages;
- *cmd++ = SVGA_CMD_DEFINE_GMR2;
- memcpy(cmd, &define_cmd, sizeof(define_cmd));
- cmd += sizeof(define_cmd) / sizeof(uint32);
+ while (num_pages > 0) {
+ unsigned long nr = min(num_pages, (unsigned
long)VMW_PPN_PER_REMAP);
+
+ remap_cmd.offsetPages = remap_pos;
+ remap_cmd.numPages = nr;
- *cmd++ = SVGA_CMD_REMAP_GMR2;
- memcpy(cmd, &remap_cmd, sizeof(remap_cmd));
- cmd += sizeof(remap_cmd) / sizeof(uint32);
+ *cmd++ = SVGA_CMD_REMAP_GMR2;
+ memcpy(cmd, &remap_cmd, sizeof(remap_cmd));
+ cmd += sizeof(remap_cmd) / sizeof(*cmd);
- for (i = 0; i < num_pages; ++i) {
- if (VMW_PPN_SIZE <= 4)
- *cmd = page_to_pfn(*pages++);
- else
- *((uint64_t *)cmd) = page_to_pfn(*pages++);
+ for (i = 0; i < nr; ++i) {
+ if (VMW_PPN_SIZE <= 4)
+ *cmd = page_to_pfn(*pages++);
+ else
+ *((uint64_t *)cmd) = page_to_pfn(*pages++);
- cmd += VMW_PPN_SIZE / sizeof(*cmd);
+ cmd += VMW_PPN_SIZE / sizeof(*cmd);
+ }
+
+ num_pages -= nr;
+ remap_pos += nr;
}
- vmw_fifo_commit(dev_priv, define_size + remap_size);
+ BUG_ON(cmd != cmd_orig + cmd_size / sizeof(*cmd));
+
+ vmw_fifo_commit(dev_priv, cmd_size);
return 0;
}
--
1.8.1.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/vmwgfx: Don't access file_priv in cursor_set when handle==0
Thanks, looks good and is
Reviewed-by: Jakob Bornecrantz
Cheers, Jakob.
On Mon, Jun 3, 2013 at 3:10 PM, wrote:
> From: Ville Syrj?l?
>
> We want to disable the cursor by calling ->cursor_set() with handle=0
> from places where we don't have a file_priv, so don't try to access it
> unless necessary.
>
> Signed-off-by: Ville Syrj?l?
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 3e3c7ab..d4607b2 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -174,7 +174,6 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc,
> struct drm_file *file_priv,
>uint32_t handle, uint32_t width, uint32_t
> height)
> {
> struct vmw_private *dev_priv = vmw_priv(crtc->dev);
> - struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile;
> struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
> struct vmw_surface *surface = NULL;
> struct vmw_dma_buffer *dmabuf = NULL;
> @@ -197,6 +196,8 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc,
> struct drm_file *file_priv,
> }
>
> if (handle) {
> + struct ttm_object_file *tfile =
> vmw_fpriv(file_priv)->tfile;
> +
> ret = vmw_user_lookup_handle(dev_priv, tfile,
> handle, &surface, &dmabuf);
> if (ret) {
> --
> 1.8.1.5
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
-- next part --
An HTML attachment was scrubbed...
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20130603/785ba177/attachment.html>
Re: [PATCH 2/3] drm/vmwgfx: Don't access file_priv in cursor_set when handle==0
Thanks, looks good and is
Reviewed-by: Jakob Bornecrantz
Cheers, Jakob.
On Mon, Jun 3, 2013 at 3:10 PM, wrote:
> From: Ville Syrjälä
>
> We want to disable the cursor by calling ->cursor_set() with handle=0
> from places where we don't have a file_priv, so don't try to access it
> unless necessary.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 3e3c7ab..d4607b2 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -174,7 +174,6 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc,
> struct drm_file *file_priv,
>uint32_t handle, uint32_t width, uint32_t
> height)
> {
> struct vmw_private *dev_priv = vmw_priv(crtc->dev);
> - struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile;
> struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
> struct vmw_surface *surface = NULL;
> struct vmw_dma_buffer *dmabuf = NULL;
> @@ -197,6 +196,8 @@ int vmw_du_crtc_cursor_set(struct drm_crtc *crtc,
> struct drm_file *file_priv,
> }
>
> if (handle) {
> + struct ttm_object_file *tfile =
> vmw_fpriv(file_priv)->tfile;
> +
> ret = vmw_user_lookup_handle(dev_priv, tfile,
> handle, &surface, &dmabuf);
> if (ret) {
> --
> 1.8.1.5
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] kms: Allow compiling libkms without Intel support
Oh, how very x86 centric of me, the changes are
Reviewed-by Jakob Bornecrantz
Cheers, Jakob.
On Fri, Apr 12, 2013 at 4:07 PM, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:
> Signed-off-by: Laurent Pinchart
> ---
> libkms/Makefile.am | 5 -
> libkms/linux.c | 6 +-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/libkms/Makefile.am b/libkms/Makefile.am
> index 215450a..518021f 100644
> --- a/libkms/Makefile.am
> +++ b/libkms/Makefile.am
> @@ -15,10 +15,13 @@ libkms_la_LIBADD = ../libdrm.la
> libkms_la_SOURCES = \
> internal.h \
> linux.c \
> - intel.c \
> dumb.c \
> api.c
>
> +if HAVE_INTEL
> +libkms_la_SOURCES += intel.c
> +endif
> +
> if HAVE_VMWGFX
> libkms_la_SOURCES += vmwgfx.c
> endif
> diff --git a/libkms/linux.c b/libkms/linux.c
> index eec0162..d160bc8 100644
> --- a/libkms/linux.c
> +++ b/libkms/linux.c
> @@ -101,8 +101,12 @@ linux_from_sysfs(int fd, struct kms_driver **out)
> if (ret)
> return ret;
>
> - if (!strcmp(name, "intel"))
> + if (0)
> + {}
> +#ifdef HAVE_INTEL
> + else if (!strcmp(name, "intel"))
> ret = intel_create(fd, out);
> +#endif
> #ifdef HAVE_VMWGFX
> else if (!strcmp(name, "vmwgfx"))
> ret = vmwgfx_create(fd, out);
> --
> Regards,
>
> Laurent Pinchart
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
-- next part --
An HTML attachment was scrubbed...
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20130412/a49d7826/attachment.html>
Re: [PATCH] kms: Allow compiling libkms without Intel support
Oh, how very x86 centric of me, the changes are
Reviewed-by Jakob Bornecrantz
Cheers, Jakob.
On Fri, Apr 12, 2013 at 4:07 PM, Laurent Pinchart <
laurent.pinch...@ideasonboard.com> wrote:
> Signed-off-by: Laurent Pinchart
> ---
> libkms/Makefile.am | 5 -
> libkms/linux.c | 6 +-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/libkms/Makefile.am b/libkms/Makefile.am
> index 215450a..518021f 100644
> --- a/libkms/Makefile.am
> +++ b/libkms/Makefile.am
> @@ -15,10 +15,13 @@ libkms_la_LIBADD = ../libdrm.la
> libkms_la_SOURCES = \
> internal.h \
> linux.c \
> - intel.c \
> dumb.c \
> api.c
>
> +if HAVE_INTEL
> +libkms_la_SOURCES += intel.c
> +endif
> +
> if HAVE_VMWGFX
> libkms_la_SOURCES += vmwgfx.c
> endif
> diff --git a/libkms/linux.c b/libkms/linux.c
> index eec0162..d160bc8 100644
> --- a/libkms/linux.c
> +++ b/libkms/linux.c
> @@ -101,8 +101,12 @@ linux_from_sysfs(int fd, struct kms_driver **out)
> if (ret)
> return ret;
>
> - if (!strcmp(name, "intel"))
> + if (0)
> + {}
> +#ifdef HAVE_INTEL
> + else if (!strcmp(name, "intel"))
> ret = intel_create(fd, out);
> +#endif
> #ifdef HAVE_VMWGFX
> else if (!strcmp(name, "vmwgfx"))
> ret = vmwgfx_create(fd, out);
> --
> Regards,
>
> Laurent Pinchart
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] kms: Make libkms.h usable in C++
On Thu, Nov 1, 2012 at 10:40 AM, Laurent Pinchart
wrote:
> Wrap the header in extern "C" { ... };.
>
> Signed-off-by: Laurent Pinchart
Reviewed-by: Jakob Bornecrantz
[PATCH] kms: Return a negative error code in kms_bo_create()
On Thu, Nov 1, 2012 at 10:38 AM, Laurent Pinchart wrote: > The function returns returns 0 on success or a negative value in case of an > error, except when given invalid attributes in which case it returns the > positive EINVAL value. Replace that with -EINVAL to allow the caller to detect > errors with a < 0 check. > > Signed-off-by: Laurent Pinchart Reviewed-by: Jakob Bornecrantz
Re: [PATCH] kms: Make libkms.h usable in C++
On Thu, Nov 1, 2012 at 10:40 AM, Laurent Pinchart
wrote:
> Wrap the header in extern "C" { ... };.
>
> Signed-off-by: Laurent Pinchart
Reviewed-by: Jakob Bornecrantz
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] kms: Return a negative error code in kms_bo_create()
On Thu, Nov 1, 2012 at 10:38 AM, Laurent Pinchart wrote: > The function returns returns 0 on success or a negative value in case of an > error, except when given invalid attributes in which case it returns the > positive EINVAL value. Replace that with -EINVAL to allow the caller to detect > errors with a < 0 check. > > Signed-off-by: Laurent Pinchart Reviewed-by: Jakob Bornecrantz ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] drm/doc: integrate crtc helper api into docbook
On Thu, Nov 1, 2012 at 2:45 PM, Daniel Vetter wrote:
> - Add the missing doc for drm_helper_move_panel_connectors_to_head.
> - Fixup any outdated stuff in existing sections. I've only looked at
> those kerneldoc headers that actually resulted in a complaint from
> the kerneldoc parser tool.
>
> v2:
> - Actually include the docbook snippet in the right patch.
> - Fix spelling fail.
>
> v3: It's now called drm_crtc_helper_set_mode, spotted by Laurent
> Pinchart.
>
> Acked-by: Laurent Pinchart
> Signed-off-by: Daniel Vetter
> ---
> Documentation/DocBook/drm.tmpl| 4 +++
> drivers/gpu/drm/drm_crtc_helper.c | 66
> +--
> 2 files changed, 46 insertions(+), 24 deletions(-)
>
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 270bc12..c2b31b9 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2106,6 +2106,10 @@ void intel_crt_init(struct drm_device *dev)
>
>
>
> +
> + Modeset Helper Functions Reference
> +!Edrivers/gpu/drm/drm_crtc_helper.c
> +
>
>
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c
> b/drivers/gpu/drm/drm_crtc_helper.c
> index 7105168..2a7a886 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -39,6 +39,17 @@
> #include
> #include
>
> +/**
> + * drm_helper_move_panel_connectors_to_head() - move panels to the front in
> the
> + * connector list
> + * @dev: drm device to operate on
> + *
> + * Some userspace presumes that the first connected connector is the main
> + * display, where it's supposed to display e.g. the login screen. For
> + * laptops, this should be the main panel. Use this function to sort all
> + * (eDP/LVDS) panels to the front of the connector list, instead of
> + * painstakingly trying to initialize them in the right order.
> + */
> void drm_helper_move_panel_connectors_to_head(struct drm_device *dev)
> {
> struct drm_connector *connector, *tmp;
> @@ -82,22 +93,21 @@ static void drm_mode_validate_flag(struct drm_connector
> *connector,
>
> /**
> * drm_helper_probe_single_connector_modes - get complete set of display
> modes
> - * @dev: DRM device
> + * @connector: connector to probe
> * @maxX: max width for modes
> * @maxY: max height for modes
> *
> * LOCKING:
> * Caller must hold mode config lock.
> *
> - * Based on @dev's mode_config layout, scan all the connectors and try to
> detect
> - * modes on them. Modes will first be added to the connector's probed_modes
> - * list, then culled (based on validity and the @maxX, @maxY parameters) and
> - * put into the normal modes list.
> - *
> - * Intended to be used either at bootup time or when major configuration
> - * changes have occurred.
> + * Based on the helper callbacks implemented by @connector try to detect all
> + * valid modes. Modes will first be added to the connector's probed_modes
> list,
> + * then culled (based on validity and the @maxX, @maxY parameters) and put
> into
> + * the normal modes list.
> *
> - * FIXME: take into account monitor limits
Should this really be removed? I'm guess it has been fixed or is not
really needed anymore but just making sure it shouldn't live somewhere
else.
Cheers, Jakob.
Re: [PATCH 2/4] drm/doc: integrate crtc helper api into docbook
On Thu, Nov 1, 2012 at 2:45 PM, Daniel Vetter wrote:
> - Add the missing doc for drm_helper_move_panel_connectors_to_head.
> - Fixup any outdated stuff in existing sections. I've only looked at
> those kerneldoc headers that actually resulted in a complaint from
> the kerneldoc parser tool.
>
> v2:
> - Actually include the docbook snippet in the right patch.
> - Fix spelling fail.
>
> v3: It's now called drm_crtc_helper_set_mode, spotted by Laurent
> Pinchart.
>
> Acked-by: Laurent Pinchart
> Signed-off-by: Daniel Vetter
> ---
> Documentation/DocBook/drm.tmpl| 4 +++
> drivers/gpu/drm/drm_crtc_helper.c | 66
> +--
> 2 files changed, 46 insertions(+), 24 deletions(-)
>
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 270bc12..c2b31b9 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2106,6 +2106,10 @@ void intel_crt_init(struct drm_device *dev)
>
>
>
> +
> + Modeset Helper Functions Reference
> +!Edrivers/gpu/drm/drm_crtc_helper.c
> +
>
>
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c
> b/drivers/gpu/drm/drm_crtc_helper.c
> index 7105168..2a7a886 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -39,6 +39,17 @@
> #include
> #include
>
> +/**
> + * drm_helper_move_panel_connectors_to_head() - move panels to the front in
> the
> + * connector list
> + * @dev: drm device to operate on
> + *
> + * Some userspace presumes that the first connected connector is the main
> + * display, where it's supposed to display e.g. the login screen. For
> + * laptops, this should be the main panel. Use this function to sort all
> + * (eDP/LVDS) panels to the front of the connector list, instead of
> + * painstakingly trying to initialize them in the right order.
> + */
> void drm_helper_move_panel_connectors_to_head(struct drm_device *dev)
> {
> struct drm_connector *connector, *tmp;
> @@ -82,22 +93,21 @@ static void drm_mode_validate_flag(struct drm_connector
> *connector,
>
> /**
> * drm_helper_probe_single_connector_modes - get complete set of display
> modes
> - * @dev: DRM device
> + * @connector: connector to probe
> * @maxX: max width for modes
> * @maxY: max height for modes
> *
> * LOCKING:
> * Caller must hold mode config lock.
> *
> - * Based on @dev's mode_config layout, scan all the connectors and try to
> detect
> - * modes on them. Modes will first be added to the connector's probed_modes
> - * list, then culled (based on validity and the @maxX, @maxY parameters) and
> - * put into the normal modes list.
> - *
> - * Intended to be used either at bootup time or when major configuration
> - * changes have occurred.
> + * Based on the helper callbacks implemented by @connector try to detect all
> + * valid modes. Modes will first be added to the connector's probed_modes
> list,
> + * then culled (based on validity and the @maxX, @maxY parameters) and put
> into
> + * the normal modes list.
> *
> - * FIXME: take into account monitor limits
Should this really be removed? I'm guess it has been fixed or is not
really needed anymore but just making sure it shouldn't live somewhere
else.
Cheers, Jakob.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
drm/vmwgfx: add MODULE_DEVICE_TABLE so vmwgfx loads at boot
On Thu, Sep 6, 2012 at 3:35 PM, Tim Gardner wrote: > Dave - I couldn't find this patch in your git repo at > git://people.freedesktop.org/~airlied/linux in the drm-next or drm-fixes > branches. > > https://patchwork.kernel.org/patch/1379071/ > > It appears to fix a real problem for Mac users - > http://bugs.launchpad.net/bugs/1039157. See > https://bugs.launchpad.net/ubuntu/oneiric/+source/linux/+bug/1039157/comments/31 > for confirmation. Thanks, I was going to point out that Dave had already fix this, but it seem you already knew about that. There is a companion patch along side with the one mentioned. http://lists.freedesktop.org/archives/dri-devel/2012-August/027124.html http://lists.freedesktop.org/archives/dri-devel/2012-August/027125.html I recommend applying both and turning the config to yes (as long as you have the kms enabled xf86-video-vmware driver installed). I RB:ed them both along with the dumb ioctl interface here: http://lists.freedesktop.org/archives/dri-devel/2012-August/027139.html > > I think it should also be 'Cc: stable at vger.kernel.org'. It applies as > far back as 2.6.32 with minor context differences. > > rtg Its probably safe, but it could mean that the driver gets loaded on distros, where in the past it hasn't tho that should be safe. Cheers, Jakob.
Re: drm/vmwgfx: add MODULE_DEVICE_TABLE so vmwgfx loads at boot
On Thu, Sep 6, 2012 at 3:35 PM, Tim Gardner wrote: > Dave - I couldn't find this patch in your git repo at > git://people.freedesktop.org/~airlied/linux in the drm-next or drm-fixes > branches. > > https://patchwork.kernel.org/patch/1379071/ > > It appears to fix a real problem for Mac users - > http://bugs.launchpad.net/bugs/1039157. See > https://bugs.launchpad.net/ubuntu/oneiric/+source/linux/+bug/1039157/comments/31 > for confirmation. Thanks, I was going to point out that Dave had already fix this, but it seem you already knew about that. There is a companion patch along side with the one mentioned. http://lists.freedesktop.org/archives/dri-devel/2012-August/027124.html http://lists.freedesktop.org/archives/dri-devel/2012-August/027125.html I recommend applying both and turning the config to yes (as long as you have the kms enabled xf86-video-vmware driver installed). I RB:ed them both along with the dumb ioctl interface here: http://lists.freedesktop.org/archives/dri-devel/2012-August/027139.html > > I think it should also be 'Cc: sta...@vger.kernel.org'. It applies as > far back as 2.6.32 with minor context differences. > > rtg Its probably safe, but it could mean that the driver gets loaded on distros, where in the past it hasn't tho that should be safe. Cheers, Jakob. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] vmwgfx: add dumb ioctl support
Thanks for doing this.
With an exception of a comment below all 3 patches are
Reviewed-by: Jakob Bornecrantz
- Original Message -
> From: Dave Airlie
>
> Testing and works with the -modesetting driver at least so far.
>
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 5 +++
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 10 +
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 73
>
> 3 files changed, 88 insertions(+)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 4d9edea..4d13bf7 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1154,6 +1154,11 @@ static struct drm_driver driver = {
> .open = vmw_driver_open,
> .preclose = vmw_preclose,
> .postclose = vmw_postclose,
> +
> + .dumb_create = vmw_dumb_create,
> + .dumb_map_offset = vmw_dumb_map_offset,
> + .dumb_destroy = vmw_dumb_destroy,
> +
> .fops = &vmwgfx_driver_fops,
> .name = VMWGFX_DRIVER_NAME,
> .desc = VMWGFX_DRIVER_DESC,
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index d0f2c07..29c984f 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -645,6 +645,16 @@ int vmw_kms_readback(struct vmw_private
> *dev_priv,
> int vmw_kms_update_layout_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
>
> +int vmw_dumb_create(struct drm_file *file_priv,
> + struct drm_device *dev,
> + struct drm_mode_create_dumb *args);
> +
> +int vmw_dumb_map_offset(struct drm_file *file_priv,
> + struct drm_device *dev, uint32_t handle,
> + uint64_t *offset);
> +int vmw_dumb_destroy(struct drm_file *file_priv,
> + struct drm_device *dev,
> + uint32_t handle);
> /**
> * Overlay control - vmwgfx_overlay.c
> */
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> index 22bf9a2..5761775 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> @@ -1917,3 +1917,76 @@ err_ref:
> vmw_resource_unreference(&res);
> return ret;
> }
> +
> +
> +int vmw_dumb_create(struct drm_file *file_priv,
> + struct drm_device *dev,
> + struct drm_mode_create_dumb *args)
> +{
> + struct vmw_private *dev_priv = vmw_priv(dev);
> + struct vmw_master *vmaster = vmw_master(file_priv->master);
> + struct vmw_user_dma_buffer *vmw_user_bo;
> + struct ttm_buffer_object *tmp;
> + int ret;
> +
> + args->pitch = args->width * args->bpp / 8;
> + args->size = args->pitch * args->height;
We talked about this on IRC, it should probably be
(args->bpp + 7) / 8
Cheers, Jakob.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] vmwgfx: add dumb ioctl support
Thanks for doing this.
With an exception of a comment below all 3 patches are
Reviewed-by: Jakob Bornecrantz
- Original Message -
> From: Dave Airlie
>
> Testing and works with the -modesetting driver at least so far.
>
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 5 +++
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 10 +
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 73
>
> 3 files changed, 88 insertions(+)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 4d9edea..4d13bf7 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1154,6 +1154,11 @@ static struct drm_driver driver = {
> .open = vmw_driver_open,
> .preclose = vmw_preclose,
> .postclose = vmw_postclose,
> +
> + .dumb_create = vmw_dumb_create,
> + .dumb_map_offset = vmw_dumb_map_offset,
> + .dumb_destroy = vmw_dumb_destroy,
> +
> .fops = &vmwgfx_driver_fops,
> .name = VMWGFX_DRIVER_NAME,
> .desc = VMWGFX_DRIVER_DESC,
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index d0f2c07..29c984f 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -645,6 +645,16 @@ int vmw_kms_readback(struct vmw_private
> *dev_priv,
> int vmw_kms_update_layout_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
>
> +int vmw_dumb_create(struct drm_file *file_priv,
> + struct drm_device *dev,
> + struct drm_mode_create_dumb *args);
> +
> +int vmw_dumb_map_offset(struct drm_file *file_priv,
> + struct drm_device *dev, uint32_t handle,
> + uint64_t *offset);
> +int vmw_dumb_destroy(struct drm_file *file_priv,
> + struct drm_device *dev,
> + uint32_t handle);
> /**
> * Overlay control - vmwgfx_overlay.c
> */
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> index 22bf9a2..5761775 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> @@ -1917,3 +1917,76 @@ err_ref:
> vmw_resource_unreference(&res);
> return ret;
> }
> +
> +
> +int vmw_dumb_create(struct drm_file *file_priv,
> + struct drm_device *dev,
> + struct drm_mode_create_dumb *args)
> +{
> + struct vmw_private *dev_priv = vmw_priv(dev);
> + struct vmw_master *vmaster = vmw_master(file_priv->master);
> + struct vmw_user_dma_buffer *vmw_user_bo;
> + struct ttm_buffer_object *tmp;
> + int ret;
> +
> + args->pitch = args->width * args->bpp / 8;
> + args->size = args->pitch * args->height;
We talked about this on IRC, it should probably be
(args->bpp + 7) / 8
Cheers, Jakob.
[PATCH] dri: Rework planar image interface
As discussed with Kristian on #wayland. Pushes the decision of components
into the dri driver giving it greater freedom to allow t to implement YUV
samplers in hardware, and which mode to use.
This interface will also allow drivers like SVGA to implement YUV surfaces
without the need to sub-allocate and instead send 3 seperate buffers
for each channel, currently not implemented.
This is only the dri interface and gallium dri state tracker parts of the
changes needed to make this change work, I haven't done the intel parts
of them and I'll do these tomorrow or monday, I wanted to get the dri-
interface changes out as fast as possible for review.
I have tested these changes on SVGA with the previus patch series I sent
out (mines the last one), and everything seem to work just as before.
Cheers, Jakob.
Signed-off-by: Jakob Bornecrantz
---
include/GL/internal/dri_interface.h| 67 +-
src/egl/drivers/dri2/egl_dri2.c| 128 ++--
src/gallium/state_trackers/dri/common/dri_screen.h |1 +
src/gallium/state_trackers/dri/drm/dri2.c | 79 +++-
src/gbm/backends/dri/gbm_dri.c | 33 ++---
5 files changed, 193 insertions(+), 115 deletions(-)
diff --git a/include/GL/internal/dri_interface.h
b/include/GL/internal/dri_interface.h
index 09f63ff..1e0f1d0 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -923,6 +923,10 @@ struct __DRIdri2ExtensionRec {
* __DRI_IMAGE_FORMAT_NONE is for images that aren't directly usable
* by the driver (YUV planar formats) but serve as a base image for
* creating sub-images for the different planes within the image.
+ *
+ * R8, GR88 and NONE should not be used with createImageFormName or
+ * createImage, and are returned by query from sub images created with
+ * createImageFromNames (NONE, see above) and fromPlane (R8 & GR88).
*/
#define __DRI_IMAGE_FORMAT_RGB565 0x1001
#define __DRI_IMAGE_FORMAT_XRGB 0x1002
@@ -937,6 +941,49 @@ struct __DRIdri2ExtensionRec {
#define __DRI_IMAGE_USE_SCANOUT0x0002
#define __DRI_IMAGE_USE_CURSOR 0x0004 /* Depricated */
+
+/**
+ * Four CC formats that matches with WL_DRM_FORMAT_* from wayland_drm.h
+ * and GBM_FORMAT_* from gbm.h, used with createImageFromNames.
+ *
+ * \since 5
+ */
+
+#define __DRI_IMAGE_FOURCC_RGB565 0x36314752
+#define __DRI_IMAGE_FOURCC_ARGB0x34325241
+#define __DRI_IMAGE_FOURCC_XRGB0x34325258
+#define __DRI_IMAGE_FOURCC_ABGR0x34324241
+#define __DRI_IMAGE_FOURCC_XBGR0x34324258
+#define __DRI_IMAGE_FOURCC_YUV410 0x39565559
+#define __DRI_IMAGE_FOURCC_YUV411 0x31315559
+#define __DRI_IMAGE_FOURCC_YUV420 0x32315559
+#define __DRI_IMAGE_FOURCC_YUV422 0x36315559
+#define __DRI_IMAGE_FOURCC_YUV444 0x34325559
+#define __DRI_IMAGE_FOURCC_NV120x3231564e
+#define __DRI_IMAGE_FOURCC_NV160x3631564e
+#define __DRI_IMAGE_FOURCC_YUYV0x56595559
+
+
+/**
+ * Queryable on images created by createImageFromNames.
+ *
+ * RGB and RGBA are may be usable directly as images but its still
+ * recommended to call fromPlanar with plane == 0.
+ *
+ * Y_U_V, Y_UV and Y_XUXV all requires call to fromPlanar to create
+ * usable sub-images, sampling from images return raw YUV data and
+ * color conversion needs to be done in the shader.
+ *
+ * \since 5
+ */
+
+#define __DRI_IMAGE_COMPONENTS_RGB 0x3001
+#define __DRI_IMAGE_COMPONENTS_RGBA0x3002
+#define __DRI_IMAGE_COMPONENTS_Y_U_V 0x3003
+#define __DRI_IMAGE_COMPONENTS_Y_UV0x3004
+#define __DRI_IMAGE_COMPONENTS_Y_XUXV 0x3005
+
+
/**
* queryImage attributes
*/
@@ -947,6 +994,7 @@ struct __DRIdri2ExtensionRec {
#define __DRI_IMAGE_ATTRIB_FORMAT 0x2003 /* available in versions 3+ */
#define __DRI_IMAGE_ATTRIB_WIDTH 0x2004 /* available in versions 4+ */
#define __DRI_IMAGE_ATTRIB_HEIGHT 0x2005
+#define __DRI_IMAGE_ATTRIB_COMPONENTS 0x2006 /* available in versions 5+ */
typedef struct __DRIimageRec __DRIimage;
typedef struct __DRIimageExtensionRec __DRIimageExtension;
@@ -984,6 +1032,19 @@ struct __DRIimageExtensionRec {
GLboolean (*validateUsage)(__DRIimage *image, unsigned int use);
/**
+* Unlike createImageFromName __DRI_IMAGE_FORMAT is not but instead
+* __DRI_IMAGE_FOURCC and strides are in bytes not pixels. Stride is
+* also per block and not per pixel (for non-RGB, see gallium blocks).
+*
+* \since 5
+*/
+ __DRIimage *(*createImageFromNames)(__DRIscreen *screen,
+ int width, int height, int fourcc,
+ int *names, int num_names,
+ int *strides, int *offsets,
+ void *loaderPrivate);
+
+ /**
* Create an image out of a sub-region of a parent image.
[PATCH] dri: Rework planar image interface
As discussed with Kristian on #wayland. Pushes the decision of components
into the dri driver giving it greater freedom to allow t to implement YUV
samplers in hardware, and which mode to use.
This interface will also allow drivers like SVGA to implement YUV surfaces
without the need to sub-allocate and instead send 3 seperate buffers
for each channel, currently not implemented.
This is only the dri interface and gallium dri state tracker parts of the
changes needed to make this change work, I haven't done the intel parts
of them and I'll do these tomorrow or monday, I wanted to get the dri-
interface changes out as fast as possible for review.
I have tested these changes on SVGA with the previus patch series I sent
out (mines the last one), and everything seem to work just as before.
Cheers, Jakob.
Signed-off-by: Jakob Bornecrantz
---
include/GL/internal/dri_interface.h| 67 +-
src/egl/drivers/dri2/egl_dri2.c| 128 ++--
src/gallium/state_trackers/dri/common/dri_screen.h |1 +
src/gallium/state_trackers/dri/drm/dri2.c | 79 +++-
src/gbm/backends/dri/gbm_dri.c | 33 ++---
5 files changed, 193 insertions(+), 115 deletions(-)
diff --git a/include/GL/internal/dri_interface.h
b/include/GL/internal/dri_interface.h
index 09f63ff..1e0f1d0 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -923,6 +923,10 @@ struct __DRIdri2ExtensionRec {
* __DRI_IMAGE_FORMAT_NONE is for images that aren't directly usable
* by the driver (YUV planar formats) but serve as a base image for
* creating sub-images for the different planes within the image.
+ *
+ * R8, GR88 and NONE should not be used with createImageFormName or
+ * createImage, and are returned by query from sub images created with
+ * createImageFromNames (NONE, see above) and fromPlane (R8 & GR88).
*/
#define __DRI_IMAGE_FORMAT_RGB565 0x1001
#define __DRI_IMAGE_FORMAT_XRGB 0x1002
@@ -937,6 +941,49 @@ struct __DRIdri2ExtensionRec {
#define __DRI_IMAGE_USE_SCANOUT0x0002
#define __DRI_IMAGE_USE_CURSOR 0x0004 /* Depricated */
+
+/**
+ * Four CC formats that matches with WL_DRM_FORMAT_* from wayland_drm.h
+ * and GBM_FORMAT_* from gbm.h, used with createImageFromNames.
+ *
+ * \since 5
+ */
+
+#define __DRI_IMAGE_FOURCC_RGB565 0x36314752
+#define __DRI_IMAGE_FOURCC_ARGB0x34325241
+#define __DRI_IMAGE_FOURCC_XRGB0x34325258
+#define __DRI_IMAGE_FOURCC_ABGR0x34324241
+#define __DRI_IMAGE_FOURCC_XBGR0x34324258
+#define __DRI_IMAGE_FOURCC_YUV410 0x39565559
+#define __DRI_IMAGE_FOURCC_YUV411 0x31315559
+#define __DRI_IMAGE_FOURCC_YUV420 0x32315559
+#define __DRI_IMAGE_FOURCC_YUV422 0x36315559
+#define __DRI_IMAGE_FOURCC_YUV444 0x34325559
+#define __DRI_IMAGE_FOURCC_NV120x3231564e
+#define __DRI_IMAGE_FOURCC_NV160x3631564e
+#define __DRI_IMAGE_FOURCC_YUYV0x56595559
+
+
+/**
+ * Queryable on images created by createImageFromNames.
+ *
+ * RGB and RGBA are may be usable directly as images but its still
+ * recommended to call fromPlanar with plane == 0.
+ *
+ * Y_U_V, Y_UV and Y_XUXV all requires call to fromPlanar to create
+ * usable sub-images, sampling from images return raw YUV data and
+ * color conversion needs to be done in the shader.
+ *
+ * \since 5
+ */
+
+#define __DRI_IMAGE_COMPONENTS_RGB 0x3001
+#define __DRI_IMAGE_COMPONENTS_RGBA0x3002
+#define __DRI_IMAGE_COMPONENTS_Y_U_V 0x3003
+#define __DRI_IMAGE_COMPONENTS_Y_UV0x3004
+#define __DRI_IMAGE_COMPONENTS_Y_XUXV 0x3005
+
+
/**
* queryImage attributes
*/
@@ -947,6 +994,7 @@ struct __DRIdri2ExtensionRec {
#define __DRI_IMAGE_ATTRIB_FORMAT 0x2003 /* available in versions 3+ */
#define __DRI_IMAGE_ATTRIB_WIDTH 0x2004 /* available in versions 4+ */
#define __DRI_IMAGE_ATTRIB_HEIGHT 0x2005
+#define __DRI_IMAGE_ATTRIB_COMPONENTS 0x2006 /* available in versions 5+ */
typedef struct __DRIimageRec __DRIimage;
typedef struct __DRIimageExtensionRec __DRIimageExtension;
@@ -984,6 +1032,19 @@ struct __DRIimageExtensionRec {
GLboolean (*validateUsage)(__DRIimage *image, unsigned int use);
/**
+* Unlike createImageFromName __DRI_IMAGE_FORMAT is not but instead
+* __DRI_IMAGE_FOURCC and strides are in bytes not pixels. Stride is
+* also per block and not per pixel (for non-RGB, see gallium blocks).
+*
+* \since 5
+*/
+ __DRIimage *(*createImageFromNames)(__DRIscreen *screen,
+ int width, int height, int fourcc,
+ int *names, int num_names,
+ int *strides, int *offsets,
+ void *loaderPrivate);
+
+ /**
* Create an image out of a sub-region of a parent image.
Re: [PATCH] drm: stop vmgfx driver explosion
- Original Message - > From: Alan Cox > > If you do a page flip with no flags set then event is NULL. If event > is NULL then the vmw_gfx driver likes to go digging into NULL and > extracts NULL->base.file_priv. > > On a modern kernel with NULL mapping protection it's just another > oops, without it there are some "intriguing" possibilities. > > What it should do is an open question but that for the driver owners > to sort out. > > Signed-off-by: Alan Cox Thanks Alan! Reviewed-by: Jakob Bornecrantz I think CC stable is in order. Cheers, Jakob. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: stop vmgfx driver explosion
- Original Message - > From: Alan Cox > > If you do a page flip with no flags set then event is NULL. If event > is NULL then the vmw_gfx driver likes to go digging into NULL and > extracts NULL->base.file_priv. > > On a modern kernel with NULL mapping protection it's just another > oops, without it there are some "intriguing" possibilities. > > What it should do is an open question but that for the driver owners > to sort out. > > Signed-off-by: Alan Cox Thanks Alan! Reviewed-by: Jakob Bornecrantz I think CC stable is in order. Cheers, Jakob.
[PATCH] drm: Check for invalid cursor flags
Signed-off-by: Jakob Bornecrantz
---
drivers/gpu/drm/drm_crtc.c |2 +-
include/drm/drm_mode.h |5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 08a7aa7..6fbfc24 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1981,7 +1981,7 @@ int drm_mode_cursor_ioctl(struct drm_device *dev,
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EINVAL;
- if (!req->flags)
+ if (!req->flags || (~DRM_MODE_CURSOR_FLAGS & req->flags))
return -EINVAL;
mutex_lock(&dev->mode_config.mutex);
diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index 5581980..3d6301b 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -359,8 +359,9 @@ struct drm_mode_mode_cmd {
struct drm_mode_modeinfo mode;
};
-#define DRM_MODE_CURSOR_BO (1<<0)
-#define DRM_MODE_CURSOR_MOVE (1<<1)
+#define DRM_MODE_CURSOR_BO 0x01
+#define DRM_MODE_CURSOR_MOVE 0x02
+#define DRM_MODE_CURSOR_FLAGS 0x03
/*
* depending on the value in flags different members are used.
--
1.7.9.5
[PATCH] drm: Check for invalid cursor flags
Signed-off-by: Jakob Bornecrantz
---
drivers/gpu/drm/drm_crtc.c |2 +-
include/drm/drm_mode.h |5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 08a7aa7..6fbfc24 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1981,7 +1981,7 @@ int drm_mode_cursor_ioctl(struct drm_device *dev,
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EINVAL;
- if (!req->flags)
+ if (!req->flags || (~DRM_MODE_CURSOR_FLAGS & req->flags))
return -EINVAL;
mutex_lock(&dev->mode_config.mutex);
diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index 5581980..3d6301b 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -359,8 +359,9 @@ struct drm_mode_mode_cmd {
struct drm_mode_modeinfo mode;
};
-#define DRM_MODE_CURSOR_BO (1<<0)
-#define DRM_MODE_CURSOR_MOVE (1<<1)
+#define DRM_MODE_CURSOR_BO 0x01
+#define DRM_MODE_CURSOR_MOVE 0x02
+#define DRM_MODE_CURSOR_FLAGS 0x03
/*
* depending on the value in flags different members are used.
--
1.7.9.5
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm: call pci_disable_device on module onload
On Fri, Jun 8, 2012 at 4:52 PM, Daniel Vetter wrote:
> Otherwise we'll nicely leak this reference counter. Now thanks to the
> awesome layering in the drm core, the enable call is done by the pci
> boilerplate in drm_pci.c. But the disable can't be done without adding
> yet another neat indirection layer just for that.
>
> So take the simple way and sprinkle pci_disable_device over all pci
> modesetting drivers.
>
> Also don't forget these dear old legacy drivers, prinkle the
> pci_disable_device call in drm_pci_exit to cover these, too.
>
> Signed-Off-by: Daniel Vetter
Looks good, one question inlined.
CC: Stable?
> ---
> ?drivers/gpu/drm/ast/ast_drv.c ? ? ? ? | ? ?2 ++
> ?drivers/gpu/drm/cirrus/cirrus_drv.c ? | ? ?2 ++
> ?drivers/gpu/drm/drm_pci.c ? ? ? ? ? ? | ? ?4 +++-
> ?drivers/gpu/drm/gma500/psb_drv.c ? ? ?| ? ?3 +++
> ?drivers/gpu/drm/i915/i915_drv.c ? ? ? | ? ?2 ++
> ?drivers/gpu/drm/mgag200/mgag200_drv.c | ? ?2 ++
> ?drivers/gpu/drm/nouveau/nouveau_drv.c | ? ?2 ++
> ?drivers/gpu/drm/radeon/radeon_drv.c ? | ? ?2 ++
> ?drivers/gpu/drm/vmwgfx/vmwgfx_drv.c ? | ? ?2 ++
> ?9 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index d0c4574..6d26e53 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -72,6 +72,8 @@ ast_pci_remove(struct pci_dev *pdev)
> ?{
> ? ? ? ?struct drm_device *dev = pci_get_drvdata(pdev);
>
> + ? ? ? pci_disable_device(dev->pdev);
> +
> ? ? ? ?drm_put_dev(dev);
> ?}
>
> diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.c
> b/drivers/gpu/drm/cirrus/cirrus_drv.c
> index d703823..d316ba3 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_drv.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_drv.c
> @@ -45,6 +45,8 @@ static void cirrus_pci_remove(struct pci_dev *pdev)
> ?{
> ? ? ? ?struct drm_device *dev = pci_get_drvdata(pdev);
>
> + ? ? ? pci_disable_device(dev->pdev);
> +
> ? ? ? ?drm_put_dev(dev);
> ?}
>
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 59e11e4..73218ac 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -455,8 +455,10 @@ void drm_pci_exit(struct drm_driver *driver, struct
> pci_driver *pdriver)
> ? ? ? ?if (driver->driver_features & DRIVER_MODESET) {
> ? ? ? ? ? ? ? ?pci_unregister_driver(pdriver);
> ? ? ? ?} else {
> - ? ? ? ? ? ? ? list_for_each_entry_safe(dev, tmp, &driver->device_list,
> driver_item)
> + ? ? ? ? ? ? ? list_for_each_entry_safe(dev, tmp, &driver->device_list,
> driver_item) {
> + ? ? ? ? ? ? ? ? ? ? ? pci_disable_device(dev->pdev);
I'm assuming that we take a ref in the enter func as well?
Cheers, Jakob.
Re: [PATCH 1/2] drm: call pci_disable_device on module onload
On Fri, Jun 8, 2012 at 4:52 PM, Daniel Vetter wrote:
> Otherwise we'll nicely leak this reference counter. Now thanks to the
> awesome layering in the drm core, the enable call is done by the pci
> boilerplate in drm_pci.c. But the disable can't be done without adding
> yet another neat indirection layer just for that.
>
> So take the simple way and sprinkle pci_disable_device over all pci
> modesetting drivers.
>
> Also don't forget these dear old legacy drivers, prinkle the
> pci_disable_device call in drm_pci_exit to cover these, too.
>
> Signed-Off-by: Daniel Vetter
Looks good, one question inlined.
CC: Stable?
> ---
> drivers/gpu/drm/ast/ast_drv.c | 2 ++
> drivers/gpu/drm/cirrus/cirrus_drv.c | 2 ++
> drivers/gpu/drm/drm_pci.c | 4 +++-
> drivers/gpu/drm/gma500/psb_drv.c | 3 +++
> drivers/gpu/drm/i915/i915_drv.c | 2 ++
> drivers/gpu/drm/mgag200/mgag200_drv.c | 2 ++
> drivers/gpu/drm/nouveau/nouveau_drv.c | 2 ++
> drivers/gpu/drm/radeon/radeon_drv.c | 2 ++
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 ++
> 9 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index d0c4574..6d26e53 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -72,6 +72,8 @@ ast_pci_remove(struct pci_dev *pdev)
> {
> struct drm_device *dev = pci_get_drvdata(pdev);
>
> + pci_disable_device(dev->pdev);
> +
> drm_put_dev(dev);
> }
>
> diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.c
> b/drivers/gpu/drm/cirrus/cirrus_drv.c
> index d703823..d316ba3 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_drv.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_drv.c
> @@ -45,6 +45,8 @@ static void cirrus_pci_remove(struct pci_dev *pdev)
> {
> struct drm_device *dev = pci_get_drvdata(pdev);
>
> + pci_disable_device(dev->pdev);
> +
> drm_put_dev(dev);
> }
>
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 59e11e4..73218ac 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -455,8 +455,10 @@ void drm_pci_exit(struct drm_driver *driver, struct
> pci_driver *pdriver)
> if (driver->driver_features & DRIVER_MODESET) {
> pci_unregister_driver(pdriver);
> } else {
> - list_for_each_entry_safe(dev, tmp, &driver->device_list,
> driver_item)
> + list_for_each_entry_safe(dev, tmp, &driver->device_list,
> driver_item) {
> + pci_disable_device(dev->pdev);
I'm assuming that we take a ref in the enter func as well?
Cheers, Jakob.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Only build test programs with make check
With my limited knowledge of automake I'm going to have to NACK this patch. Most of these programs are used during driver bring up to test things out, often we also modify them a bit to suit our need. If this patch doesn't make this any harder then disregard my NACK. Cheers Jakob. - Original Message - > Signed-off-by: Matt Turner > --- > tests/kmstest/Makefile.am |2 +- > tests/modeprint/Makefile.am |2 +- > tests/modetest/Makefile.am |2 +- > tests/radeon/Makefile.am|2 +- > tests/vbltest/Makefile.am |2 +- > 5 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/tests/kmstest/Makefile.am b/tests/kmstest/Makefile.am > index ae562a1..10b9ef3 100644 > --- a/tests/kmstest/Makefile.am > +++ b/tests/kmstest/Makefile.am > @@ -3,7 +3,7 @@ AM_CFLAGS = \ > -I$(top_srcdir)/libkms/ \ > -I$(top_srcdir) > > -noinst_PROGRAMS = \ > +check_PROGRAMS = \ > kmstest > > kmstest_SOURCES = \ > diff --git a/tests/modeprint/Makefile.am > b/tests/modeprint/Makefile.am > index c4862ac..4291dc5 100644 > --- a/tests/modeprint/Makefile.am > +++ b/tests/modeprint/Makefile.am > @@ -2,7 +2,7 @@ AM_CFLAGS = \ > -I$(top_srcdir)/include/drm \ > -I$(top_srcdir) > > -noinst_PROGRAMS = \ > +check_PROGRAMS = \ > modeprint > > modeprint_SOURCES = \ > diff --git a/tests/modetest/Makefile.am b/tests/modetest/Makefile.am > index 2191242..dd43d0c 100644 > --- a/tests/modetest/Makefile.am > +++ b/tests/modetest/Makefile.am > @@ -4,7 +4,7 @@ AM_CFLAGS = \ > -I$(top_srcdir) \ > $(CAIRO_CFLAGS) > > -noinst_PROGRAMS = \ > +check_PROGRAMS = \ > modetest > > modetest_SOURCES = \ > diff --git a/tests/radeon/Makefile.am b/tests/radeon/Makefile.am > index 1775669..d4f6755 100644 > --- a/tests/radeon/Makefile.am > +++ b/tests/radeon/Makefile.am > @@ -4,7 +4,7 @@ AM_CFLAGS = \ > > LDADD = $(top_builddir)/libdrm.la > > -noinst_PROGRAMS = \ > +check_PROGRAMS = \ > radeon_ttm > > radeon_ttm_SOURCES = \ > diff --git a/tests/vbltest/Makefile.am b/tests/vbltest/Makefile.am > index 77f9037..5886bd1 100644 > --- a/tests/vbltest/Makefile.am > +++ b/tests/vbltest/Makefile.am > @@ -2,7 +2,7 @@ AM_CFLAGS = \ > -I$(top_srcdir)/include/drm \ > -I$(top_srcdir) > > -noinst_PROGRAMS = \ > +check_PROGRAMS = \ > vbltest > > vbltest_SOURCES = \ > -- > 1.7.3.4 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] Only build test programs with make check
With my limited knowledge of automake I'm going to have to NACK this patch. Most of these programs are used during driver bring up to test things out, often we also modify them a bit to suit our need. If this patch doesn't make this any harder then disregard my NACK. Cheers Jakob. - Original Message - > Signed-off-by: Matt Turner > --- > tests/kmstest/Makefile.am |2 +- > tests/modeprint/Makefile.am |2 +- > tests/modetest/Makefile.am |2 +- > tests/radeon/Makefile.am|2 +- > tests/vbltest/Makefile.am |2 +- > 5 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/tests/kmstest/Makefile.am b/tests/kmstest/Makefile.am > index ae562a1..10b9ef3 100644 > --- a/tests/kmstest/Makefile.am > +++ b/tests/kmstest/Makefile.am > @@ -3,7 +3,7 @@ AM_CFLAGS = \ > -I$(top_srcdir)/libkms/ \ > -I$(top_srcdir) > > -noinst_PROGRAMS = \ > +check_PROGRAMS = \ > kmstest > > kmstest_SOURCES = \ > diff --git a/tests/modeprint/Makefile.am > b/tests/modeprint/Makefile.am > index c4862ac..4291dc5 100644 > --- a/tests/modeprint/Makefile.am > +++ b/tests/modeprint/Makefile.am > @@ -2,7 +2,7 @@ AM_CFLAGS = \ > -I$(top_srcdir)/include/drm \ > -I$(top_srcdir) > > -noinst_PROGRAMS = \ > +check_PROGRAMS = \ > modeprint > > modeprint_SOURCES = \ > diff --git a/tests/modetest/Makefile.am b/tests/modetest/Makefile.am > index 2191242..dd43d0c 100644 > --- a/tests/modetest/Makefile.am > +++ b/tests/modetest/Makefile.am > @@ -4,7 +4,7 @@ AM_CFLAGS = \ > -I$(top_srcdir) \ > $(CAIRO_CFLAGS) > > -noinst_PROGRAMS = \ > +check_PROGRAMS = \ > modetest > > modetest_SOURCES = \ > diff --git a/tests/radeon/Makefile.am b/tests/radeon/Makefile.am > index 1775669..d4f6755 100644 > --- a/tests/radeon/Makefile.am > +++ b/tests/radeon/Makefile.am > @@ -4,7 +4,7 @@ AM_CFLAGS = \ > > LDADD = $(top_builddir)/libdrm.la > > -noinst_PROGRAMS = \ > +check_PROGRAMS = \ > radeon_ttm > > radeon_ttm_SOURCES = \ > diff --git a/tests/vbltest/Makefile.am b/tests/vbltest/Makefile.am > index 77f9037..5886bd1 100644 > --- a/tests/vbltest/Makefile.am > +++ b/tests/vbltest/Makefile.am > @@ -2,7 +2,7 @@ AM_CFLAGS = \ > -I$(top_srcdir)/include/drm \ > -I$(top_srcdir) > > -noinst_PROGRAMS = \ > +check_PROGRAMS = \ > vbltest > > vbltest_SOURCES = \ > -- > 1.7.3.4 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >
Re: [PATCH 1/5] drm/vgem: virtual GEM provider
- Original Message - > From: Adam Jackson > > This is about as minimal of a virtual GEM service as possible. My > plan is to use this with non-native-3D hardware for buffer sharing > between X and DRI. > > The current drisw winsys assumes an unmodified X server, which means > it's hopelessly inefficient for both the push direction of > SwapBuffers/DrawPixels and the pull direction of > GLX_EXT_texture_from_pixmap. I'm still working through the details > of what the xserver support will look like, but in broad strokes it's > "use vgem for CreatePixmap and optionally the shadowfb". > > Obviously alpha quality, mostly looking for feedback on the approach > or any glaring bugs. Eventually I'd like to see solutions for > sharing gem objects between drm devices and/or the dma_buf API, but > that's a ways down the road. > > Signed-off-by: Adam Jackson Any reason why you are not using the dumb_bo interface? I at least would like to be able to offer vgem on the vmwgfx device when the host has disabled 3D. Cheers, Jakob. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/5] drm/vgem: virtual GEM provider
- Original Message - > From: Adam Jackson > > This is about as minimal of a virtual GEM service as possible. My > plan is to use this with non-native-3D hardware for buffer sharing > between X and DRI. > > The current drisw winsys assumes an unmodified X server, which means > it's hopelessly inefficient for both the push direction of > SwapBuffers/DrawPixels and the pull direction of > GLX_EXT_texture_from_pixmap. I'm still working through the details > of what the xserver support will look like, but in broad strokes it's > "use vgem for CreatePixmap and optionally the shadowfb". > > Obviously alpha quality, mostly looking for feedback on the approach > or any glaring bugs. Eventually I'd like to see solutions for > sharing gem objects between drm devices and/or the dma_buf API, but > that's a ways down the road. > > Signed-off-by: Adam Jackson Any reason why you are not using the dumb_bo interface? I at least would like to be able to offer vgem on the vmwgfx device when the host has disabled 3D. Cheers, Jakob.
Re: [RFC] drm: atomic mode set API
- Original Message -
> Many of us really want (and need) a way to set the whole display
> configuration atomically, as well as test a global config.
>
> In talking with Rob and Alex here at ELC a bit, I think this may be
> enough:
>
> diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
> index 2a2acda..2864b02 100644
> --- a/include/drm/drm_mode.h
> +++ b/include/drm/drm_mode.h
> @@ -157,6 +157,26 @@ struct drm_mode_get_plane_res {
> __u32 count_planes;
> };
>
> +#define DRM_SET_CONFIG_TEST (1<<0) /* don't change the config, just test it
> for validity */
> +
> +struct drm_mode_set_config {
> + __u64 crtcs;
> + __u64 crtc_fbs;
> + __u64 crtc_xpos; /* array of x coords for crtcs */
> + __u64 crtc_ypos; /* array of y coords for crtcs */
> + __u32 count_crtcs;
> +
> + __u64 plane_sets; /* array of set_plane structs */
> +
> + __u32 count_planes;
> +
> + __u64 connectors;
> + __u64 connector_modes;
> + __u32 count_connectors;
How do you do mirroring? Why not just send down a array of
drm_mode_crtc?
> +
> + __u32 flags;
> +};
> +
> #define DRM_MODE_ENCODER_NONE0
> #define DRM_MODE_ENCODER_DAC 1
> #define DRM_MODE_ENCODER_TMDS2
>
> This allows you to bind a bunch of fbs to crtcs with independent
> positions, as well as set a bunch of planes to specific fbs and
> layouts. Finally, it lets you change the connector config at the
> same time, with a flag to simply test a config instead of
> actually setting it.
>
> Any comments? Do we also need to set gamma or other properties as
> part of this? What about cursors?
I would like to see properties added as well.
Cheers, Jakob.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC] drm: atomic mode set API
- Original Message -
> Many of us really want (and need) a way to set the whole display
> configuration atomically, as well as test a global config.
>
> In talking with Rob and Alex here at ELC a bit, I think this may be
> enough:
>
> diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
> index 2a2acda..2864b02 100644
> --- a/include/drm/drm_mode.h
> +++ b/include/drm/drm_mode.h
> @@ -157,6 +157,26 @@ struct drm_mode_get_plane_res {
> __u32 count_planes;
> };
>
> +#define DRM_SET_CONFIG_TEST (1<<0) /* don't change the config, just test it
> for validity */
> +
> +struct drm_mode_set_config {
> + __u64 crtcs;
> + __u64 crtc_fbs;
> + __u64 crtc_xpos; /* array of x coords for crtcs */
> + __u64 crtc_ypos; /* array of y coords for crtcs */
> + __u32 count_crtcs;
> +
> + __u64 plane_sets; /* array of set_plane structs */
> +
> + __u32 count_planes;
> +
> + __u64 connectors;
> + __u64 connector_modes;
> + __u32 count_connectors;
How do you do mirroring? Why not just send down a array of
drm_mode_crtc?
> +
> + __u32 flags;
> +};
> +
> #define DRM_MODE_ENCODER_NONE0
> #define DRM_MODE_ENCODER_DAC 1
> #define DRM_MODE_ENCODER_TMDS2
>
> This allows you to bind a bunch of fbs to crtcs with independent
> positions, as well as set a bunch of planes to specific fbs and
> layouts. Finally, it lets you change the connector config at the
> same time, with a flag to simply test a config instead of
> actually setting it.
>
> Any comments? Do we also need to set gamma or other properties as
> part of this? What about cursors?
I would like to see properties added as well.
Cheers, Jakob.
Re: [PATCH] [trivial] drm: Fix typo in vmwgfx_drv.c
Reviewed-by: Jakob Bornecrantz
- Original Message -
> Correct spelling "unsuported" to "unsupported" in
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>
> Signed-off-by: Masanari Iida
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index f390f5f..2d6f573 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -430,7 +430,7 @@ static int vmw_driver_load(struct drm_device
> *dev, unsigned long chipset)
> svga_id = vmw_read(dev_priv, SVGA_REG_ID);
> if (svga_id != SVGA_ID_2) {
> ret = -ENOSYS;
> - DRM_ERROR("Unsuported SVGA ID 0x%x\n", svga_id);
> + DRM_ERROR("Unsupported SVGA ID 0x%x\n", svga_id);
> mutex_unlock(&dev_priv->hw_mutex);
> goto out_err0;
> }
> --
> 1.7.6.5
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] [trivial] drm: Fix typo in vmwgfx_drv.c
Reviewed-by: Jakob Bornecrantz
- Original Message -
> Correct spelling "unsuported" to "unsupported" in
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>
> Signed-off-by: Masanari Iida
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index f390f5f..2d6f573 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -430,7 +430,7 @@ static int vmw_driver_load(struct drm_device
> *dev, unsigned long chipset)
> svga_id = vmw_read(dev_priv, SVGA_REG_ID);
> if (svga_id != SVGA_ID_2) {
> ret = -ENOSYS;
> - DRM_ERROR("Unsuported SVGA ID 0x%x\n", svga_id);
> + DRM_ERROR("Unsupported SVGA ID 0x%x\n", svga_id);
> mutex_unlock(&dev_priv->hw_mutex);
> goto out_err0;
> }
> --
> 1.7.6.5
Re: [RFC PATCH] vmwgfx: Fix assignment in vmw_framebuffer_create_handle
- Original Message - > On 01/27/2012 03:41 PM, Jakob Bornecrantz wrote: > > - Original Message - > >> On 01/27/2012 03:24 PM, Jakob Bornecrantz wrote: > >>> I was asking around and this seems to only be used by X when it > >>> starts and we want to preserve the contents of the screen. That > >>> feature is implemented by the X driver. So we need to figure how > >>> we > >>> want to solve it. > >>> > >>> Either way this fix should probably go into this RC series, not > >>> sure if we need to send this to stable, since we are not leaking > >>> data to userspace (check drm_mode_getfb), but we might as well. > >>> > >>> Reviewed-by: Jakob Bornecrantz > >> But shouldn't we return the *real* handle. Not 0?? > > > > Yeah, you are right. Not sure we have user_handle in the stable > > kernels tho. > > We do AFAICT. > > > > > We need to change not only the X driver but also all the other > > userspace components since at least libkms doesn't create shared > > buffers. > > Libkms is not used anymore, at least not in the X server driver. I was more referring to Plymouth and the like. > > And if the thing that is running before X is using fbdev > > this wont do much good either since fbdev is not backed by a > > fb in the way other drivers do it. > > OK. Then I suggest we just return 0 here, and go ahead with the > current patch, and when we've figured out how to do the handover, > we bump kernel minor and return a proper handle. Does that sound > OK? Sounds good to me. Cheers, Jakob. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH] vmwgfx: Fix assignment in vmw_framebuffer_create_handle
- Original Message - > On 01/27/2012 03:41 PM, Jakob Bornecrantz wrote: > > - Original Message - > >> On 01/27/2012 03:24 PM, Jakob Bornecrantz wrote: > >>> I was asking around and this seems to only be used by X when it > >>> starts and we want to preserve the contents of the screen. That > >>> feature is implemented by the X driver. So we need to figure how > >>> we > >>> want to solve it. > >>> > >>> Either way this fix should probably go into this RC series, not > >>> sure if we need to send this to stable, since we are not leaking > >>> data to userspace (check drm_mode_getfb), but we might as well. > >>> > >>> Reviewed-by: Jakob Bornecrantz > >> But shouldn't we return the *real* handle. Not 0?? > > > > Yeah, you are right. Not sure we have user_handle in the stable > > kernels tho. > > We do AFAICT. > > > > > We need to change not only the X driver but also all the other > > userspace components since at least libkms doesn't create shared > > buffers. > > Libkms is not used anymore, at least not in the X server driver. I was more referring to Plymouth and the like. > > And if the thing that is running before X is using fbdev > > this wont do much good either since fbdev is not backed by a > > fb in the way other drivers do it. > > OK. Then I suggest we just return 0 here, and go ahead with the > current patch, and when we've figured out how to do the handover, > we bump kernel minor and return a proper handle. Does that sound > OK? Sounds good to me. Cheers, Jakob.
Re: [RFC PATCH] vmwgfx: Fix assignment in vmw_framebuffer_create_handle
- Original Message - > On 01/27/2012 03:24 PM, Jakob Bornecrantz wrote: > > I was asking around and this seems to only be used by X when it > > starts and we want to preserve the contents of the screen. That > > feature is implemented by the X driver. So we need to figure how we > > want to solve it. > > > > Either way this fix should probably go into this RC series, not > > sure if we need to send this to stable, since we are not leaking > > data to userspace (check drm_mode_getfb), but we might as well. > > > > Reviewed-by: Jakob Bornecrantz > > But shouldn't we return the *real* handle. Not 0?? Yeah, you are right. Not sure we have user_handle in the stable kernels tho. We need to change not only the X driver but also all the other userspace components since at least libkms doesn't create shared buffers. And if the thing that is running before X is using fbdev this wont do much good either since fbdev is not backed by a fb in the way other drivers do it. Cheers, Jakob. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH] vmwgfx: Fix assignment in vmw_framebuffer_create_handle
- Original Message - > On 01/27/2012 03:24 PM, Jakob Bornecrantz wrote: > > I was asking around and this seems to only be used by X when it > > starts and we want to preserve the contents of the screen. That > > feature is implemented by the X driver. So we need to figure how we > > want to solve it. > > > > Either way this fix should probably go into this RC series, not > > sure if we need to send this to stable, since we are not leaking > > data to userspace (check drm_mode_getfb), but we might as well. > > > > Reviewed-by: Jakob Bornecrantz > > But shouldn't we return the *real* handle. Not 0?? Yeah, you are right. Not sure we have user_handle in the stable kernels tho. We need to change not only the X driver but also all the other userspace components since at least libkms doesn't create shared buffers. And if the thing that is running before X is using fbdev this wont do much good either since fbdev is not backed by a fb in the way other drivers do it. Cheers, Jakob.
Re: [RFC PATCH] vmwgfx: Fix assignment in vmw_framebuffer_create_handle
I was asking around and this seems to only be used by X when it
starts and we want to preserve the contents of the screen. That
feature is implemented by the X driver. So we need to figure how we
want to solve it.
Either way this fix should probably go into this RC series, not
sure if we need to send this to stable, since we are not leaking
data to userspace (check drm_mode_getfb), but we might as well.
Reviewed-by: Jakob Bornecrantz
Cheers, Jakob.
- Original Message -
> Ryan,
>
> Thanks for pointing this out. Unfortunately there seems to be two
> bugs here, the one you pointed out and the fact that we set the handle
> to zero, when it probably should be set to struct
> vmw_framebuffer::user_handle.
>
> Jakob, can you comment on this?
>
> /Thomas
>
> On 01/27/2012 07:25 AM, Ryan Mallon wrote:
> > The assignment of handle in vmw_framebuffer_create_handle doesn't
> > actually do anything useful and is incorrectly assigning an integer
> > value to a pointer argument. It appears that this is a typo and
> > should
> > be dereferencing handle rather than assigning to it directly.
> >
> > Signed-off-by: Ryan Mallon
> > ---
> >
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > index 0af6ebd..b66ef0e 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -378,7 +378,7 @@ int vmw_framebuffer_create_handle(struct
> > drm_framebuffer *fb,
> > unsigned int *handle)
> > {
> > if (handle)
> > - handle = 0;
> > + *handle = 0;
> >
> > return 0;
> > }
> >
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH] vmwgfx: Fix assignment in vmw_framebuffer_create_handle
I was asking around and this seems to only be used by X when it
starts and we want to preserve the contents of the screen. That
feature is implemented by the X driver. So we need to figure how we
want to solve it.
Either way this fix should probably go into this RC series, not
sure if we need to send this to stable, since we are not leaking
data to userspace (check drm_mode_getfb), but we might as well.
Reviewed-by: Jakob Bornecrantz
Cheers, Jakob.
- Original Message -
> Ryan,
>
> Thanks for pointing this out. Unfortunately there seems to be two
> bugs here, the one you pointed out and the fact that we set the handle
> to zero, when it probably should be set to struct
> vmw_framebuffer::user_handle.
>
> Jakob, can you comment on this?
>
> /Thomas
>
> On 01/27/2012 07:25 AM, Ryan Mallon wrote:
> > The assignment of handle in vmw_framebuffer_create_handle doesn't
> > actually do anything useful and is incorrectly assigning an integer
> > value to a pointer argument. It appears that this is a typo and
> > should
> > be dereferencing handle rather than assigning to it directly.
> >
> > Signed-off-by: Ryan Mallon
> > ---
> >
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > index 0af6ebd..b66ef0e 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -378,7 +378,7 @@ int vmw_framebuffer_create_handle(struct
> > drm_framebuffer *fb,
> > unsigned int *handle)
> > {
> > if (handle)
> > - handle = 0;
> > + *handle = 0;
> >
> > return 0;
> > }
> >
>
>
Re: [RFC] drm: implement DRM_IOCTL_MODE_SETROTATION
- Original Message - > From: Paulo Zanoni > > This ioctl is used to signal the drivers that the screen is rotated, > not to make the drivers rotate the screen. > - add a driver-specific "rotation_set" function > - implement Intel's rotation_set by setting the right values to the >PIPECONF registers. > > The idea is that when user-space does rotation, it can call this > ioctl to inform the Kernel that we have a rotation. This feature is > needed by the KVMr feature of VPro. > > Signed-off-by: Paulo Zanoni Couldn't this be done by just adding a property instead of a ioctl? Also the name make it sound like you are setting the rotation and not just setting a hint, so the word hit should probably be in the name. Cheers Jakob. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC] drm: implement DRM_IOCTL_MODE_SETROTATION
- Original Message - > From: Paulo Zanoni > > This ioctl is used to signal the drivers that the screen is rotated, > not to make the drivers rotate the screen. > - add a driver-specific "rotation_set" function > - implement Intel's rotation_set by setting the right values to the >PIPECONF registers. > > The idea is that when user-space does rotation, it can call this > ioctl to inform the Kernel that we have a rotation. This feature is > needed by the KVMr feature of VPro. > > Signed-off-by: Paulo Zanoni Couldn't this be done by just adding a property instead of a ioctl? Also the name make it sound like you are setting the rotation and not just setting a hint, so the word hit should probably be in the name. Cheers Jakob.
Re: [PATCH] vmwgfx: Use kcalloc instead of kzalloc to allocate array
Reviewed-by: Jakob Bornecrantz
- Original Message -
> The advantage of kcalloc is, that will prevent integer overflows
> which could result from the multiplication of number of elements
> and size and it is also a bit nicer to read.
>
> The semantic patch that makes this change is available
> in https://lkml.org/lkml/2011/11/25/107
>
> Signed-off-by: Thomas Meyer
> ---
>
> diff -u -p a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c 2011-11-13
> 11:07:24.343455126 +0100
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c 2011-11-28
> 19:50:07.413502824 +0100
> @@ -140,7 +140,7 @@ int vmw_present_ioctl(struct drm_device
> goto out_clips;
> }
>
> - clips = kzalloc(num_clips * sizeof(*clips), GFP_KERNEL);
> + clips = kcalloc(num_clips, sizeof(*clips), GFP_KERNEL);
> if (clips == NULL) {
> DRM_ERROR("Failed to allocate clip rect list.\n");
> ret = -ENOMEM;
> @@ -232,7 +232,7 @@ int vmw_present_readback_ioctl(struct dr
> goto out_clips;
> }
>
> - clips = kzalloc(num_clips * sizeof(*clips), GFP_KERNEL);
> + clips = kcalloc(num_clips, sizeof(*clips), GFP_KERNEL);
> if (clips == NULL) {
> DRM_ERROR("Failed to allocate clip rect list.\n");
> ret = -ENOMEM;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] vmwgfx: Use kcalloc instead of kzalloc to allocate array
Reviewed-by: Jakob Bornecrantz
- Original Message -
> The advantage of kcalloc is, that will prevent integer overflows
> which could result from the multiplication of number of elements
> and size and it is also a bit nicer to read.
>
> The semantic patch that makes this change is available
> in https://lkml.org/lkml/2011/11/25/107
>
> Signed-off-by: Thomas Meyer
> ---
>
> diff -u -p a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c 2011-11-13
> 11:07:24.343455126 +0100
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c 2011-11-28
> 19:50:07.413502824 +0100
> @@ -140,7 +140,7 @@ int vmw_present_ioctl(struct drm_device
> goto out_clips;
> }
>
> - clips = kzalloc(num_clips * sizeof(*clips), GFP_KERNEL);
> + clips = kcalloc(num_clips, sizeof(*clips), GFP_KERNEL);
> if (clips == NULL) {
> DRM_ERROR("Failed to allocate clip rect list.\n");
> ret = -ENOMEM;
> @@ -232,7 +232,7 @@ int vmw_present_readback_ioctl(struct dr
> goto out_clips;
> }
>
> - clips = kzalloc(num_clips * sizeof(*clips), GFP_KERNEL);
> + clips = kcalloc(num_clips, sizeof(*clips), GFP_KERNEL);
> if (clips == NULL) {
> DRM_ERROR("Failed to allocate clip rect list.\n");
> ret = -ENOMEM;
Re: [PATCH 1/2] vmwgfx: Emulate depth 32 framebuffers
- Original Message -
> On Sat, Oct 22, 2011 at 10:29:33AM +0200, Thomas Hellstrom wrote:
> > From: Jakob Bornecrantz
> >
> > Signed-off-by: Jakob Bornecrantz
> > Signed-off-by: Thomas Hellstrom
> > ---
> > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 10 +-
> > 1 files changed, 9 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > index 39b99db..00ec619 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -679,6 +679,7 @@ static int do_dmabuf_define_gmrfb(struct
> > drm_file *file_priv,
> > struct vmw_private *dev_priv,
> > struct vmw_framebuffer *framebuffer)
> > {
> > + int depth = framebuffer->base.depth;
> > size_t fifo_size;
> > int ret;
> >
> > @@ -687,6 +688,13 @@ static int do_dmabuf_define_gmrfb(struct
> > drm_file *file_priv,
> > SVGAFifoCmdDefineGMRFB body;
> > } *cmd;
> >
> > + /* Emulate RGBA support, contrary to svga_reg.h this is not
> > +* supported by hosts. This is only a problem if we are reading
>
> Uh, what if it becomes supported at some point? Should there be some
> check against the host version?
>
> (Thinking that some user might be running this older driver with a
> newer host that des support 32 - won't that cause issues?)
We can add a check then, from the point of view of userspace 32 bit
framebuffers works fine. The problem is with the readback ioctl where
the readback pixels alpha will be clobbered. We also don't support
depths of 30 R10G10B10X2.
If we add support for depth 32 and/or depth 30 formats we can add
params to tell userspace about that, right now there isn't really a
point to them since they will always return not supported and we
need to do any work around in userspace anyways.
Cheers, Jakob.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] vmwgfx: Emulate depth 32 framebuffers
- Original Message -
> On Sat, Oct 22, 2011 at 10:29:33AM +0200, Thomas Hellstrom wrote:
> > From: Jakob Bornecrantz
> >
> > Signed-off-by: Jakob Bornecrantz
> > Signed-off-by: Thomas Hellstrom
> > ---
> > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 10 +-
> > 1 files changed, 9 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > index 39b99db..00ec619 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -679,6 +679,7 @@ static int do_dmabuf_define_gmrfb(struct
> > drm_file *file_priv,
> > struct vmw_private *dev_priv,
> > struct vmw_framebuffer *framebuffer)
> > {
> > + int depth = framebuffer->base.depth;
> > size_t fifo_size;
> > int ret;
> >
> > @@ -687,6 +688,13 @@ static int do_dmabuf_define_gmrfb(struct
> > drm_file *file_priv,
> > SVGAFifoCmdDefineGMRFB body;
> > } *cmd;
> >
> > + /* Emulate RGBA support, contrary to svga_reg.h this is not
> > +* supported by hosts. This is only a problem if we are reading
>
> Uh, what if it becomes supported at some point? Should there be some
> check against the host version?
>
> (Thinking that some user might be running this older driver with a
> newer host that des support 32 - won't that cause issues?)
We can add a check then, from the point of view of userspace 32 bit
framebuffers works fine. The problem is with the readback ioctl where
the readback pixels alpha will be clobbered. We also don't support
depths of 30 R10G10B10X2.
If we add support for depth 32 and/or depth 30 formats we can add
params to tell userspace about that, right now there isn't really a
point to them since they will always return not supported and we
need to do any work around in userspace anyways.
Cheers, Jakob.
Re: vmwgfx + libkms on VMware hardware 8
- Original Message - > Thanks for taking interesting in our driver. > > - Original Message - > > Hi, > > > > I 'm trying to test KMS functionality of latest vmwgfx module. > > I tried the code from > > http://permalink.gmane.org/gmane.comp.video.dri.devel/42908 > > > > as a simple test case, but all I get is a black screen instead > > of white as would be expected. > > I'm able to reproduce the bug at my end, quick debugging shows > that the commands seems to be sent to the host. I'll try and > get to the bottom of this once I done a couple of other tasks. I found the bug and have pushed a fix to the standalone repository. > > > > > Setup: > > VMWare Workstation 8.0.0 > > Host & Guest distro: kubuntu oneiric (11.10) > > libdrm: 2.4.26+git20111003.c82ef03e from xorg-edgers ppa > > vmwgfx module: latest git version from freedesktop.org > > > > dmesg output (vmwgfx initialization / capabilities) > > > > [ 3.696893] vmwgfx :00:0f.0: PCI INT A -> GSI 16 (level, low) > > -> IRQ 16 > > [ 3.697188] [vmwgfx] Capabilities: > > [ 3.697191] [vmwgfx] Rect copy. > > [ 3.697193] [vmwgfx] Cursor. > > [ 3.697196] [vmwgfx] Cursor bypass. > > [ 3.697198] [vmwgfx] Cursor bypass 2. > > [ 3.697200] [vmwgfx] 8bit emulation. > > [ 3.697202] [vmwgfx] Alpha cursor. > > [ 3.697204] [vmwgfx] 3D. > > [ 3.697206] [vmwgfx] Extended Fifo. > > [ 3.697208] [vmwgfx] Multimon. > > [ 3.697210] [vmwgfx] Pitchlock. > > [ 3.697212] [vmwgfx] Irq mask. > > [ 3.697214] [vmwgfx] Display Topology. > > [ 3.697216] [vmwgfx] GMR. > > [ 3.697218] [vmwgfx] Traces. > > [ 3.697220] [vmwgfx] GMR2. > > [ 3.697222] [vmwgfx] Screen Object 2. > > [ 3.697224] [vmwgfx] Max GMR ids is 64 > > [ 3.697226] [vmwgfx] Max GMR descriptors is 4096 > > [ 3.697229] [vmwgfx] Max number of GMR pages is 196608 > > [ 3.697231] [vmwgfx] Max dedicated hypervisor surface memory is > > 786432 kiB > > [ 3.697234] [vmwgfx] VRAM at 0xd000 size is 131072 kiB > > [ 3.697236] [vmwgfx] MMIO at 0xc880 size is 2048 kiB > > [ 3.697239] [vmwgfx] global init. > > [ 3.700262] [vmwgfx] width 640 > > [ 3.700276] [vmwgfx] height 480 > > [ 3.700289] [vmwgfx] bpp 32 > > [ 3.704308] [vmwgfx] Fifo max 0x0020 min 0x1000 cap > > 0x077f > > [ 3.728297] [vmwgfx] Screen objects system initialized > > [ 3.728303] [vmwgfx] Detected device 3D availability. > > [ 3.728401] [vmwgfx] Initialized vmwgfx 2.1.0 20110927 for > > :00:0f.0 on minor 0 > > > > I 'm trying with ./modeset -s 4@6:1024x768 > > > > Running the above under strace shows that all underlying ioctls > > succeed, so it looks like this code should work. > > > > It looks like the update command is not sent to the card because if > > I > > add the parameter svga.forceTraces = "TRUE" to the .vmx file of the > > VM, I see a white rectangle as expected (the mode still doesn't > > look > > right though, it looks like the size of the rectangle is 640x480 > > instead of 1024x768) > > Does the UI resize? If you haven't set the option for the UI to > resize itself when the guest changes size it wont and will scale > the picture. I recommend that you install the cairo dev packages as it will print a test pattern that will help you see how large the screen is. > > > > > I also get in the vmware.log when I run the above: > > > > 2011-10-18T19:21:30.849+02:00| vcpu-0| I120: MKS enabling SVGA > > 2011-10-18T19:21:30.909+02:00| vcpu-0| I120: Guest display topology > > changed: numDisplays 0 > > 2011-10-18T19:21:30.910+02:00| vcpu-0| I120: Guest display topology > > changed: numDisplays 0 > > 2011-10-18T19:21:30.934+02:00| vcpu-0| I120: Guest display topology > > changed: numDisplays 0 > > 2011-10-18T19:21:30.935+02:00| vcpu-0| I120: Guest display topology > > changed: numDisplays 0 > > 2011-10-18T19:21:30.957+02:00| vcpu-0| I120: Guest display topology > > changed: numDisplays 0 > > 2011-10-18T19:21:30.974+02:00| mks| W110: MKS-EmuSVGA: Unsupported > > GMR image format 0x2020 > > This looks suspicious. This was the problem btw. > > > 2011-10-18T19:21:38.837+02:00| vcpu-0| I120: Guest display topology > > changed: numDisplays 0 > > 2011-10-18T19:21:38.940+02:00| mks| I120: MKS disabling SVGA > > > > Any ideas on what may be going wrong? VMWare would make a pretty > > cool > > platform to experiment with several features of the modern open > > graphics stack like wayland, plymouth, flickerless boot, integrated > > kernel debugger, multiseat etc etc. > > > > I 'll be happy to provide more debugging information on this > > problem > > as needed. Cheers, Jakob. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
vmwgfx + libkms on VMware hardware 8
- Original Message - > Thanks for taking interesting in our driver. > > - Original Message - > > Hi, > > > > I 'm trying to test KMS functionality of latest vmwgfx module. > > I tried the code from > > http://permalink.gmane.org/gmane.comp.video.dri.devel/42908 > > > > as a simple test case, but all I get is a black screen instead > > of white as would be expected. > > I'm able to reproduce the bug at my end, quick debugging shows > that the commands seems to be sent to the host. I'll try and > get to the bottom of this once I done a couple of other tasks. I found the bug and have pushed a fix to the standalone repository. > > > > > Setup: > > VMWare Workstation 8.0.0 > > Host & Guest distro: kubuntu oneiric (11.10) > > libdrm: 2.4.26+git20111003.c82ef03e from xorg-edgers ppa > > vmwgfx module: latest git version from freedesktop.org > > > > dmesg output (vmwgfx initialization / capabilities) > > > > [ 3.696893] vmwgfx :00:0f.0: PCI INT A -> GSI 16 (level, low) > > -> IRQ 16 > > [ 3.697188] [vmwgfx] Capabilities: > > [ 3.697191] [vmwgfx] Rect copy. > > [ 3.697193] [vmwgfx] Cursor. > > [ 3.697196] [vmwgfx] Cursor bypass. > > [ 3.697198] [vmwgfx] Cursor bypass 2. > > [ 3.697200] [vmwgfx] 8bit emulation. > > [ 3.697202] [vmwgfx] Alpha cursor. > > [ 3.697204] [vmwgfx] 3D. > > [ 3.697206] [vmwgfx] Extended Fifo. > > [ 3.697208] [vmwgfx] Multimon. > > [ 3.697210] [vmwgfx] Pitchlock. > > [ 3.697212] [vmwgfx] Irq mask. > > [ 3.697214] [vmwgfx] Display Topology. > > [ 3.697216] [vmwgfx] GMR. > > [ 3.697218] [vmwgfx] Traces. > > [ 3.697220] [vmwgfx] GMR2. > > [ 3.697222] [vmwgfx] Screen Object 2. > > [ 3.697224] [vmwgfx] Max GMR ids is 64 > > [ 3.697226] [vmwgfx] Max GMR descriptors is 4096 > > [ 3.697229] [vmwgfx] Max number of GMR pages is 196608 > > [ 3.697231] [vmwgfx] Max dedicated hypervisor surface memory is > > 786432 kiB > > [ 3.697234] [vmwgfx] VRAM at 0xd000 size is 131072 kiB > > [ 3.697236] [vmwgfx] MMIO at 0xc880 size is 2048 kiB > > [ 3.697239] [vmwgfx] global init. > > [ 3.700262] [vmwgfx] width 640 > > [ 3.700276] [vmwgfx] height 480 > > [ 3.700289] [vmwgfx] bpp 32 > > [ 3.704308] [vmwgfx] Fifo max 0x0020 min 0x1000 cap > > 0x077f > > [ 3.728297] [vmwgfx] Screen objects system initialized > > [ 3.728303] [vmwgfx] Detected device 3D availability. > > [ 3.728401] [vmwgfx] Initialized vmwgfx 2.1.0 20110927 for > > :00:0f.0 on minor 0 > > > > I 'm trying with ./modeset -s 4 at 6:1024x768 > > > > Running the above under strace shows that all underlying ioctls > > succeed, so it looks like this code should work. > > > > It looks like the update command is not sent to the card because if > > I > > add the parameter svga.forceTraces = "TRUE" to the .vmx file of the > > VM, I see a white rectangle as expected (the mode still doesn't > > look > > right though, it looks like the size of the rectangle is 640x480 > > instead of 1024x768) > > Does the UI resize? If you haven't set the option for the UI to > resize itself when the guest changes size it wont and will scale > the picture. I recommend that you install the cairo dev packages as it will print a test pattern that will help you see how large the screen is. > > > > > I also get in the vmware.log when I run the above: > > > > 2011-10-18T19:21:30.849+02:00| vcpu-0| I120: MKS enabling SVGA > > 2011-10-18T19:21:30.909+02:00| vcpu-0| I120: Guest display topology > > changed: numDisplays 0 > > 2011-10-18T19:21:30.910+02:00| vcpu-0| I120: Guest display topology > > changed: numDisplays 0 > > 2011-10-18T19:21:30.934+02:00| vcpu-0| I120: Guest display topology > > changed: numDisplays 0 > > 2011-10-18T19:21:30.935+02:00| vcpu-0| I120: Guest display topology > > changed: numDisplays 0 > > 2011-10-18T19:21:30.957+02:00| vcpu-0| I120: Guest display topology > > changed: numDisplays 0 > > 2011-10-18T19:21:30.974+02:00| mks| W110: MKS-EmuSVGA: Unsupported > > GMR image format 0x2020 > > This looks suspicious. This was the problem btw. > > > 2011-10-18T19:21:38.837+02:00| vcpu-0| I120: Guest display topology > > changed: numDisplays 0 > > 2011-10-18T19:21:38.940+02:00| mks| I120: MKS disabling SVGA > > > > Any ideas on what may be going wrong? VMWare would make a pretty > > cool > > platform to experiment with several features of the modern open > > graphics stack like wayland, plymouth, flickerless boot, integrated > > kernel debugger, multiseat etc etc. > > > > I 'll be happy to provide more debugging information on this > > problem > > as needed. Cheers, Jakob.
Re: vmwgfx + libkms on VMware hardware 8
Thanks for taking interesting in our driver. - Original Message - > Hi, > > I 'm trying to test KMS functionality of latest vmwgfx module. > I tried the code from > http://permalink.gmane.org/gmane.comp.video.dri.devel/42908 > > as a simple test case, but all I get is a black screen instead > of white as would be expected. I'm able to reproduce the bug at my end, quick debugging shows that the commands seems to be sent to the host. I'll try and get to the bottom of this once I done a couple of other tasks. > > Setup: > VMWare Workstation 8.0.0 > Host & Guest distro: kubuntu oneiric (11.10) > libdrm: 2.4.26+git20111003.c82ef03e from xorg-edgers ppa > vmwgfx module: latest git version from freedesktop.org > > dmesg output (vmwgfx initialization / capabilities) > > [ 3.696893] vmwgfx :00:0f.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16 > [ 3.697188] [vmwgfx] Capabilities: > [ 3.697191] [vmwgfx] Rect copy. > [ 3.697193] [vmwgfx] Cursor. > [ 3.697196] [vmwgfx] Cursor bypass. > [ 3.697198] [vmwgfx] Cursor bypass 2. > [ 3.697200] [vmwgfx] 8bit emulation. > [ 3.697202] [vmwgfx] Alpha cursor. > [ 3.697204] [vmwgfx] 3D. > [ 3.697206] [vmwgfx] Extended Fifo. > [ 3.697208] [vmwgfx] Multimon. > [ 3.697210] [vmwgfx] Pitchlock. > [ 3.697212] [vmwgfx] Irq mask. > [ 3.697214] [vmwgfx] Display Topology. > [ 3.697216] [vmwgfx] GMR. > [ 3.697218] [vmwgfx] Traces. > [ 3.697220] [vmwgfx] GMR2. > [ 3.697222] [vmwgfx] Screen Object 2. > [ 3.697224] [vmwgfx] Max GMR ids is 64 > [ 3.697226] [vmwgfx] Max GMR descriptors is 4096 > [ 3.697229] [vmwgfx] Max number of GMR pages is 196608 > [ 3.697231] [vmwgfx] Max dedicated hypervisor surface memory is 786432 kiB > [ 3.697234] [vmwgfx] VRAM at 0xd000 size is 131072 kiB > [ 3.697236] [vmwgfx] MMIO at 0xc880 size is 2048 kiB > [ 3.697239] [vmwgfx] global init. > [ 3.700262] [vmwgfx] width 640 > [ 3.700276] [vmwgfx] height 480 > [ 3.700289] [vmwgfx] bpp 32 > [ 3.704308] [vmwgfx] Fifo max 0x0020 min 0x1000 cap 0x077f > [ 3.728297] [vmwgfx] Screen objects system initialized > [ 3.728303] [vmwgfx] Detected device 3D availability. > [ 3.728401] [vmwgfx] Initialized vmwgfx 2.1.0 20110927 for :00:0f.0 on > minor 0 > > I 'm trying with ./modeset -s 4@6:1024x768 > > Running the above under strace shows that all underlying ioctls > succeed, so it looks like this code should work. > > It looks like the update command is not sent to the card because if I > add the parameter svga.forceTraces = "TRUE" to the .vmx file of the > VM, I see a white rectangle as expected (the mode still doesn't look > right though, it looks like the size of the rectangle is 640x480 > instead of 1024x768) Does the UI resize? If you haven't set the option for the UI to resize itself when the guest changes size it wont and will scale the picture. > > I also get in the vmware.log when I run the above: > > 2011-10-18T19:21:30.849+02:00| vcpu-0| I120: MKS enabling SVGA > 2011-10-18T19:21:30.909+02:00| vcpu-0| I120: Guest display topology changed: > numDisplays 0 > 2011-10-18T19:21:30.910+02:00| vcpu-0| I120: Guest display topology changed: > numDisplays 0 > 2011-10-18T19:21:30.934+02:00| vcpu-0| I120: Guest display topology changed: > numDisplays 0 > 2011-10-18T19:21:30.935+02:00| vcpu-0| I120: Guest display topology changed: > numDisplays 0 > 2011-10-18T19:21:30.957+02:00| vcpu-0| I120: Guest display topology changed: > numDisplays 0 > 2011-10-18T19:21:30.974+02:00| mks| W110: MKS-EmuSVGA: Unsupported GMR image > format 0x2020 This looks suspicious. > 2011-10-18T19:21:38.837+02:00| vcpu-0| I120: Guest display topology changed: > numDisplays 0 > 2011-10-18T19:21:38.940+02:00| mks| I120: MKS disabling SVGA > > Any ideas on what may be going wrong? VMWare would make a pretty cool > platform to experiment with several features of the modern open > graphics stack like wayland, plymouth, flickerless boot, integrated > kernel debugger, multiseat etc etc. > > I 'll be happy to provide more debugging information on this problem > as needed. Cheers, Jakob. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
vmwgfx + libkms on VMware hardware 8
Thanks for taking interesting in our driver. - Original Message - > Hi, > > I 'm trying to test KMS functionality of latest vmwgfx module. > I tried the code from > http://permalink.gmane.org/gmane.comp.video.dri.devel/42908 > > as a simple test case, but all I get is a black screen instead > of white as would be expected. I'm able to reproduce the bug at my end, quick debugging shows that the commands seems to be sent to the host. I'll try and get to the bottom of this once I done a couple of other tasks. > > Setup: > VMWare Workstation 8.0.0 > Host & Guest distro: kubuntu oneiric (11.10) > libdrm: 2.4.26+git20111003.c82ef03e from xorg-edgers ppa > vmwgfx module: latest git version from freedesktop.org > > dmesg output (vmwgfx initialization / capabilities) > > [ 3.696893] vmwgfx :00:0f.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16 > [ 3.697188] [vmwgfx] Capabilities: > [ 3.697191] [vmwgfx] Rect copy. > [ 3.697193] [vmwgfx] Cursor. > [ 3.697196] [vmwgfx] Cursor bypass. > [ 3.697198] [vmwgfx] Cursor bypass 2. > [ 3.697200] [vmwgfx] 8bit emulation. > [ 3.697202] [vmwgfx] Alpha cursor. > [ 3.697204] [vmwgfx] 3D. > [ 3.697206] [vmwgfx] Extended Fifo. > [ 3.697208] [vmwgfx] Multimon. > [ 3.697210] [vmwgfx] Pitchlock. > [ 3.697212] [vmwgfx] Irq mask. > [ 3.697214] [vmwgfx] Display Topology. > [ 3.697216] [vmwgfx] GMR. > [ 3.697218] [vmwgfx] Traces. > [ 3.697220] [vmwgfx] GMR2. > [ 3.697222] [vmwgfx] Screen Object 2. > [ 3.697224] [vmwgfx] Max GMR ids is 64 > [ 3.697226] [vmwgfx] Max GMR descriptors is 4096 > [ 3.697229] [vmwgfx] Max number of GMR pages is 196608 > [ 3.697231] [vmwgfx] Max dedicated hypervisor surface memory is 786432 kiB > [ 3.697234] [vmwgfx] VRAM at 0xd000 size is 131072 kiB > [ 3.697236] [vmwgfx] MMIO at 0xc880 size is 2048 kiB > [ 3.697239] [vmwgfx] global init. > [ 3.700262] [vmwgfx] width 640 > [ 3.700276] [vmwgfx] height 480 > [ 3.700289] [vmwgfx] bpp 32 > [ 3.704308] [vmwgfx] Fifo max 0x0020 min 0x1000 cap 0x077f > [ 3.728297] [vmwgfx] Screen objects system initialized > [ 3.728303] [vmwgfx] Detected device 3D availability. > [ 3.728401] [vmwgfx] Initialized vmwgfx 2.1.0 20110927 for :00:0f.0 on > minor 0 > > I 'm trying with ./modeset -s 4 at 6:1024x768 > > Running the above under strace shows that all underlying ioctls > succeed, so it looks like this code should work. > > It looks like the update command is not sent to the card because if I > add the parameter svga.forceTraces = "TRUE" to the .vmx file of the > VM, I see a white rectangle as expected (the mode still doesn't look > right though, it looks like the size of the rectangle is 640x480 > instead of 1024x768) Does the UI resize? If you haven't set the option for the UI to resize itself when the guest changes size it wont and will scale the picture. > > I also get in the vmware.log when I run the above: > > 2011-10-18T19:21:30.849+02:00| vcpu-0| I120: MKS enabling SVGA > 2011-10-18T19:21:30.909+02:00| vcpu-0| I120: Guest display topology changed: > numDisplays 0 > 2011-10-18T19:21:30.910+02:00| vcpu-0| I120: Guest display topology changed: > numDisplays 0 > 2011-10-18T19:21:30.934+02:00| vcpu-0| I120: Guest display topology changed: > numDisplays 0 > 2011-10-18T19:21:30.935+02:00| vcpu-0| I120: Guest display topology changed: > numDisplays 0 > 2011-10-18T19:21:30.957+02:00| vcpu-0| I120: Guest display topology changed: > numDisplays 0 > 2011-10-18T19:21:30.974+02:00| mks| W110: MKS-EmuSVGA: Unsupported GMR image > format 0x2020 This looks suspicious. > 2011-10-18T19:21:38.837+02:00| vcpu-0| I120: Guest display topology changed: > numDisplays 0 > 2011-10-18T19:21:38.940+02:00| mks| I120: MKS disabling SVGA > > Any ideas on what may be going wrong? VMWare would make a pretty cool > platform to experiment with several features of the modern open > graphics stack like wayland, plymouth, flickerless boot, integrated > kernel debugger, multiseat etc etc. > > I 'll be happy to provide more debugging information on this problem > as needed. Cheers, Jakob.
[PATCH 06/25] vmwgfx: Some comments and BUG_ON
On Thu, Sep 29, 2011 at 11:42 PM, Konrad Rzeszutek Wilk wrote: > On Wed, Sep 28, 2011 at 04:10:02PM +0200, Thomas Hellstrom wrote: >> From: Jakob Bornecrantz >> >> Signed-off-by: Jakob Bornecrantz >> Reviewed-by: Thomas Hellstrom >> --- >> ?drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | ? ?5 + >> ?1 files changed, 5 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c >> b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c >> index df1c7c5..207f595 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c >> @@ -680,6 +680,9 @@ static int vmw_resize_cmd_bounce(struct vmw_sw_context >> *sw_context, >> ? * Creates a fence object and submits a command stream marker. >> ? * If this fails for some reason, We sync the fifo and return NULL. >> ? * It is then safe to fence buffers with a NULL pointer. >> + * >> + * If @p_handle is not NULL @file_priv must also not be not NULL. Creates > > not be not? > I think you meant: > > "must also not be NULL" > > >> + * a userspace handle if @p_handle is not NULL, does not if otherwise. > > "does not if otherwise".. how about ", otherwise not." Thanks I'll fix that. Cheers Jakob.
[PATCH 05/25] vmwgfx: Make sure the reserved area is at the start of vram
On Thu, Sep 29, 2011 at 11:42 PM, Konrad Rzeszutek Wilk wrote: > On Wed, Sep 28, 2011 at 04:10:01PM +0200, Thomas Hellstrom wrote: >> From: Jakob Bornecrantz >> >> Signed-off-by: Jakob Bornecrantz >> Reviewed-by: Thomas Hellstrom >> --- >> ?drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | ? ?5 - >> ?1 files changed, 4 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> index c14eb76..8ac6cee 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> @@ -716,7 +716,10 @@ static int vmw_surface_dmabuf_pin(struct >> vmw_framebuffer *vfb) >> ? ? ? struct vmw_framebuffer_surface *vfbs = >> ? ? ? ? ? ? ? vmw_framebuffer_to_vfbs(&vfb->base); >> ? ? ? unsigned long size = vfbs->base.base.pitch * vfbs->base.base.height; >> - ? ? int ret; >> + ? ? struct ttm_placement ne_placement = vmw_vram_ne_placement; >> + ? ? int ret = 0; > > So why the 'int ret = 0' ? That looks like it belongs to > a different patch? It doesn't do anything and is not a part of any later patch, then again its okay to be paranoid. Cheers Jakob.
[PATCH 10/25] vmwgfx: Refactor common display unit functions to shared file
On Thu, Sep 29, 2011 at 11:50 PM, Konrad Rzeszutek Wilk
wrote:
> On Wed, Sep 28, 2011 at 04:10:06PM +0200, Thomas Hellstrom wrote:
>> From: Jakob Bornecrantz
>>
>> More preparation for Screen Object support.
>>
>> Signed-off-by: Jakob Bornecrantz
>> Signed-off-by: Thomas Hellstrom
>> ---
>> ?drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | ?238 +++
>> ?drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | ? 31 -
>> ?drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | ?268
>> ++-
>> ?3 files changed, 282 insertions(+), 255 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>> index 68c6351..0c4179b 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>> @@ -1152,3 +1152,241 @@ u32 vmw_get_vblank_counter(struct drm_device *dev,
>> int crtc)
>> ?{
>> ? ? ? return 0;
>> ?}
>> +
>> +
>> +/*
>> + * Small shared kms functions.
>> + */
>> +
>> +int vmw_du_update_layout(struct vmw_private *dev_priv, unsigned num,
>> + ? ? ? ? ? ? ? ? ? ? ?struct drm_vmw_rect *rects)
>> +{
>> + ? ? struct drm_device *dev = dev_priv->dev;
>> + ? ? struct vmw_display_unit *du;
>> + ? ? struct drm_connector *con;
>> + ? ? int i;
>> +
>> + ? ? mutex_lock(&dev->mode_config.mutex);
>> +
>> +#if 0
>> + ? ? DRM_INFO("%s: new layout ", __func__);
>> + ? ? for (i = 0; i < (int)num; i++)
>
> Would it be easier to make 'i' be 'unsigned int' ?
>
>> + ? ? ? ? ? ? DRM_INFO("(%i, %i %ux%u) ", rects[i].x, rects[i].y,
>> + ? ? ? ? ? ? ? ? ? ? ?rects[i].w, rects[i].h);
>> + ? ? DRM_INFO("\n");
>> +#else
>> + ? ? (void)i;
>
> ?
> What does that do?
>
> [edit: Ah, you are moving the code, so the patch looks fine then.
> Thought I am still confused by this invocation - perhaps it makes sense
> to clean this part of the code in another patch?]
The "i" variable is only used in the commented code, and the (void)i; statement
hides the "unused variable error".
Cheers Jakob.
[PATCH 12/25] vmwgfx: Add screen object support
On Fri, Sep 30, 2011 at 12:05 AM, Konrad Rzeszutek Wilk
wrote:
> On Wed, Sep 28, 2011 at 04:10:08PM +0200, Thomas Hellstrom wrote:
>> From: Jakob Bornecrantz
>>
>> Signed-off-by: Jakob Bornecrantz
>> Signed-off-by: Thomas Hellstrom
>> ---
>> ?drivers/gpu/drm/vmwgfx/Makefile ? ? ?| ? ?2 +-
>> ?drivers/gpu/drm/vmwgfx/vmwgfx_drv.c ?| ? 34 ++-
>> ?drivers/gpu/drm/vmwgfx/vmwgfx_drv.h ?| ? ?1 +
>> ?drivers/gpu/drm/vmwgfx/vmwgfx_kms.c ?| ?165 +-
>> ?drivers/gpu/drm/vmwgfx/vmwgfx_kms.h ?| ? 10 +
>> ?drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c ?| ? ?5 +-
>> ?drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | ?566
>> ++
>> ?7 files changed, 752 insertions(+), 31 deletions(-)
>> ?create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
>>
>> diff --git a/drivers/gpu/drm/vmwgfx/Makefile
>> b/drivers/gpu/drm/vmwgfx/Makefile
>> index e13a118..586869c 100644
>> --- a/drivers/gpu/drm/vmwgfx/Makefile
>> +++ b/drivers/gpu/drm/vmwgfx/Makefile
>> @@ -5,6 +5,6 @@ vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o
>> vmwgfx_drv.o \
>> ? ? ? ? ? vmwgfx_fb.o vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_buffer.o \
>> ? ? ? ? ? vmwgfx_fifo.o vmwgfx_irq.o vmwgfx_ldu.o vmwgfx_ttm_glue.o \
>> ? ? ? ? ? vmwgfx_overlay.o vmwgfx_marker.o vmwgfx_gmrid_manager.o \
>> - ? ? ? ? vmwgfx_fence.o vmwgfx_dmabuf.o
>> + ? ? ? ? vmwgfx_fence.o vmwgfx_dmabuf.o vmwgfx_scrn.o
>>
>> ?obj-$(CONFIG_DRM_VMWGFX) := vmwgfx.o
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> index d4829cb..d1e1325 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> @@ -451,22 +451,28 @@ static int vmw_driver_load(struct drm_device *dev,
>> unsigned long chipset)
>> ? ? ? dev_priv->fman = vmw_fence_manager_init(dev_priv);
>> ? ? ? if (unlikely(dev_priv->fman == NULL))
>> ? ? ? ? ? ? ? goto out_no_fman;
>> +
>> + ? ? /* Need to start the fifo to check if we can do screen objects */
>> + ? ? ret = vmw_3d_resource_inc(dev_priv, true);
>> + ? ? if (unlikely(ret != 0))
>> + ? ? ? ? ? ? goto out_no_fifo;
>> + ? ? vmw_kms_save_vga(dev_priv);
>> + ? ? DRM_INFO("%s", vmw_fifo_have_3d(dev_priv) ?
>> + ? ? ? ? ? ? ?"Detected device 3D availability.\n" :
>> + ? ? ? ? ? ? ?"Detected no device 3D availability.\n");
>
> You could just do:
> ? ? ? ?DRM_INFO("Detected %s 3D availability\n", vmw_fifo_have_3d(dev_priv) ?
> ? ? ? ? ? ? ? ? "device" : "no device");
>
> but I see you are moving code, so that perhaps belongs to another patch.
You are right, I'll add that.
Cheers Jakob.
Re: [PATCH 06/25] vmwgfx: Some comments and BUG_ON
On Thu, Sep 29, 2011 at 11:42 PM, Konrad Rzeszutek Wilk wrote: > On Wed, Sep 28, 2011 at 04:10:02PM +0200, Thomas Hellstrom wrote: >> From: Jakob Bornecrantz >> >> Signed-off-by: Jakob Bornecrantz >> Reviewed-by: Thomas Hellstrom >> --- >> drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 5 + >> 1 files changed, 5 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c >> b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c >> index df1c7c5..207f595 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c >> @@ -680,6 +680,9 @@ static int vmw_resize_cmd_bounce(struct vmw_sw_context >> *sw_context, >> * Creates a fence object and submits a command stream marker. >> * If this fails for some reason, We sync the fifo and return NULL. >> * It is then safe to fence buffers with a NULL pointer. >> + * >> + * If @p_handle is not NULL @file_priv must also not be not NULL. Creates > > not be not? > I think you meant: > > "must also not be NULL" > > >> + * a userspace handle if @p_handle is not NULL, does not if otherwise. > > "does not if otherwise".. how about ", otherwise not." Thanks I'll fix that. Cheers Jakob. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/25] vmwgfx: Make sure the reserved area is at the start of vram
On Thu, Sep 29, 2011 at 11:42 PM, Konrad Rzeszutek Wilk wrote: > On Wed, Sep 28, 2011 at 04:10:01PM +0200, Thomas Hellstrom wrote: >> From: Jakob Bornecrantz >> >> Signed-off-by: Jakob Bornecrantz >> Reviewed-by: Thomas Hellstrom >> --- >> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 5 - >> 1 files changed, 4 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> index c14eb76..8ac6cee 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> @@ -716,7 +716,10 @@ static int vmw_surface_dmabuf_pin(struct >> vmw_framebuffer *vfb) >> struct vmw_framebuffer_surface *vfbs = >> vmw_framebuffer_to_vfbs(&vfb->base); >> unsigned long size = vfbs->base.base.pitch * vfbs->base.base.height; >> - int ret; >> + struct ttm_placement ne_placement = vmw_vram_ne_placement; >> + int ret = 0; > > So why the 'int ret = 0' ? That looks like it belongs to > a different patch? It doesn't do anything and is not a part of any later patch, then again its okay to be paranoid. Cheers Jakob. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/25] vmwgfx: Refactor common display unit functions to shared file
On Thu, Sep 29, 2011 at 11:50 PM, Konrad Rzeszutek Wilk
wrote:
> On Wed, Sep 28, 2011 at 04:10:06PM +0200, Thomas Hellstrom wrote:
>> From: Jakob Bornecrantz
>>
>> More preparation for Screen Object support.
>>
>> Signed-off-by: Jakob Bornecrantz
>> Signed-off-by: Thomas Hellstrom
>> ---
>> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 238 +++
>> drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 31 -
>> drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 268
>> ++-
>> 3 files changed, 282 insertions(+), 255 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>> index 68c6351..0c4179b 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>> @@ -1152,3 +1152,241 @@ u32 vmw_get_vblank_counter(struct drm_device *dev,
>> int crtc)
>> {
>> return 0;
>> }
>> +
>> +
>> +/*
>> + * Small shared kms functions.
>> + */
>> +
>> +int vmw_du_update_layout(struct vmw_private *dev_priv, unsigned num,
>> + struct drm_vmw_rect *rects)
>> +{
>> + struct drm_device *dev = dev_priv->dev;
>> + struct vmw_display_unit *du;
>> + struct drm_connector *con;
>> + int i;
>> +
>> + mutex_lock(&dev->mode_config.mutex);
>> +
>> +#if 0
>> + DRM_INFO("%s: new layout ", __func__);
>> + for (i = 0; i < (int)num; i++)
>
> Would it be easier to make 'i' be 'unsigned int' ?
>
>> + DRM_INFO("(%i, %i %ux%u) ", rects[i].x, rects[i].y,
>> + rects[i].w, rects[i].h);
>> + DRM_INFO("\n");
>> +#else
>> + (void)i;
>
> ?
> What does that do?
>
> [edit: Ah, you are moving the code, so the patch looks fine then.
> Thought I am still confused by this invocation - perhaps it makes sense
> to clean this part of the code in another patch?]
The "i" variable is only used in the commented code, and the (void)i; statement
hides the "unused variable error".
Cheers Jakob.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 12/25] vmwgfx: Add screen object support
On Fri, Sep 30, 2011 at 12:05 AM, Konrad Rzeszutek Wilk
wrote:
> On Wed, Sep 28, 2011 at 04:10:08PM +0200, Thomas Hellstrom wrote:
>> From: Jakob Bornecrantz
>>
>> Signed-off-by: Jakob Bornecrantz
>> Signed-off-by: Thomas Hellstrom
>> ---
>> drivers/gpu/drm/vmwgfx/Makefile | 2 +-
>> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 34 ++-
>> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 +
>> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 165 +-
>> drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 10 +
>> drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 5 +-
>> drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 566
>> ++
>> 7 files changed, 752 insertions(+), 31 deletions(-)
>> create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
>>
>> diff --git a/drivers/gpu/drm/vmwgfx/Makefile
>> b/drivers/gpu/drm/vmwgfx/Makefile
>> index e13a118..586869c 100644
>> --- a/drivers/gpu/drm/vmwgfx/Makefile
>> +++ b/drivers/gpu/drm/vmwgfx/Makefile
>> @@ -5,6 +5,6 @@ vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o
>> vmwgfx_drv.o \
>> vmwgfx_fb.o vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_buffer.o \
>> vmwgfx_fifo.o vmwgfx_irq.o vmwgfx_ldu.o vmwgfx_ttm_glue.o \
>> vmwgfx_overlay.o vmwgfx_marker.o vmwgfx_gmrid_manager.o \
>> - vmwgfx_fence.o vmwgfx_dmabuf.o
>> + vmwgfx_fence.o vmwgfx_dmabuf.o vmwgfx_scrn.o
>>
>> obj-$(CONFIG_DRM_VMWGFX) := vmwgfx.o
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> index d4829cb..d1e1325 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> @@ -451,22 +451,28 @@ static int vmw_driver_load(struct drm_device *dev,
>> unsigned long chipset)
>> dev_priv->fman = vmw_fence_manager_init(dev_priv);
>> if (unlikely(dev_priv->fman == NULL))
>> goto out_no_fman;
>> +
>> + /* Need to start the fifo to check if we can do screen objects */
>> + ret = vmw_3d_resource_inc(dev_priv, true);
>> + if (unlikely(ret != 0))
>> + goto out_no_fifo;
>> + vmw_kms_save_vga(dev_priv);
>> + DRM_INFO("%s", vmw_fifo_have_3d(dev_priv) ?
>> + "Detected device 3D availability.\n" :
>> + "Detected no device 3D availability.\n");
>
> You could just do:
> DRM_INFO("Detected %s 3D availability\n", vmw_fifo_have_3d(dev_priv) ?
> "device" : "no device");
>
> but I see you are moving code, so that perhaps belongs to another patch.
You are right, I'll add that.
Cheers Jakob.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC] drm: add overlays as first class KMS objects
On Wed, Apr 27, 2011 at 11:12 PM, Jesse Barnes
wrote:
> On Wed, 27 Apr 2011 14:19:05 +0200
> Daniel Vetter wrote:
>
>> Hi Jesse,
>>
>> I like it. It's a bit of a chicken-egg api design problem, but if a
>> proof-of-concept
>> implementation exists for an embedded chip plus something to check whether
>> it's good enough to implement Xv on ancient hw (intel overlay or for the qemu
>> kms driver, if that could do this), that should be good enough.
>>
>> A few comments on the ioctl interface.
>
> Thanks for checking it out.
>
>> > +/* Overlays blend with or override other bits on the CRTC */
>> > +struct drm_mode_set_overlay {
>> > + ? ? ? __u32 overlay_id;
>> > + ? ? ? __u32 crtc_id;
>> > + ? ? ? __u32 fb_id; /* contains surface format type */
>> > +
>> > + ? ? ? __u32 crtc_x, crtc_y;
>> > + ? ? ? __u32 x, y;
>> > +
>> > + ? ? ? /* FIXME: color key/mask, scaling, z-order, other? */
>> > +};
>>
>> I think scaling is required, you can't implement Xv without that. The
>> problem is some arbitraray
>> hw range restrictions, but returning -EINVAL if they're violated looks okay.
>> So
>>
>> float scale_x, scale_y;
>
> Ok, I'll collect more fields based on this thread and make this
> structure bigger. ?Still not sure how to handle arbitrary blend and
> z-order restrictions without passing a tree around though...
What we send over the ioctl interface (in vmwgfx) to control the
overlays is this:
/**
* struct drm_vmw_control_stream_arg
*
* @stream_id: Stearm to control
* @enabled: If false all following arguments are ignored.
* @handle: Handle to buffer for getting data from.
* @format: Format of the overlay as understood by the host.
* @width: Width of the overlay.
* @height: Height of the overlay.
* @size: Size of the overlay in bytes.
* @pitch: Array of pitches, the two last are only used for YUV12 formats.
* @offset: Offset from start of dma buffer to overlay.
* @src: Source rect, must be within the defined area above.
* @dst: Destination rect, x and y may be negative.
*
* Argument to the DRM_VMW_CONTROL_STREAM Ioctl.
*/
struct drm_vmw_control_stream_arg {
uint32_t stream_id;
uint32_t enabled;
uint32_t flags;
uint32_t color_key;
uint32_t handle;
uint32_t offset;
int32_t format;
uint32_t size;
uint32_t width;
uint32_t height;
uint32_t pitch[3];
uint32_t pad64;
struct drm_vmw_rect src;
struct drm_vmw_rect dst;
};
The command contains information describing the source "framebuffer"
as well as the data for controlling the overlay. The args are by a
amazing coincidence is almost 1:1 what we also send down to the host.
The biggest change here from what you proposed is that we send two
rects describing from where within the buffer we get the data and to
where it should go. My vote is to do that as well (makes my life
easier at least).
Tho I guess that color_key and flags could be made into a property.
Cheers Jakob.
>
>>
>> should be good enough. For the remaining things (color conversion,
>> blending, ...) I don't think
>> there's any generic way to map hw capabilities (without going insane).
>> I think a bunch of
>> properties (with standard stuff for gamma, color key, ...) would be
>> perfect for that. If we
>> set the defaults such that it Just Works for Xv (after setting the
>> color key, if present), that
>> would be great!
>
> Yeah, properties for those is probably the way to go.
>
>>
>> > +struct drm_mode_get_overlay {
>> > + ? ? ? __u64 format_type_ptr;
>> > + ? ? ? __u32 overlay_id;
>> > +
>> > + ? ? ? __u32 crtc_id;
>> > + ? ? ? __u32 fb_id;
>> > +
>> > + ? ? ? __u32 crtc_x, crtc_y;
>> > + ? ? ? __u32 x, y;
>> > +
>> > + ? ? ? __u32 possible_crtcs;
>> > + ? ? ? __u32 gamma_size;
>> > +
>> > + ? ? ? __u32 count_format_types;
>> > +};
>>
>> Imo the real problem with formats is stride restrictions and other hw
>> restrictions (tiling, ...).
>> ARM/v4l people seem to want that to be in the kernel so that they can
>> e.g. dma decoded
>> video streams directly to the gpu (also for other stuff). Perhaps we
>> want to extend the
>> create_dumb_gem_object ioctl for that use case, but I'm not too
>> convinced that this belongs
>> into the kernel.
>
> I think it's a bit like handling tiling formats; for the most part the
> kernel doesn't care because it's not reading or writing the data, but
> it does need to know the format when programming certain regs. ?So I
> don't think we can avoid having surface format information passed in
> when creating an fb for an overlay. ?And if we do that we may as well
> enumerate the types we support when overlay info is fetched to make
> portability for app code a little easier.
>
> Jesse
Re: [RFC] drm: add overlays as first class KMS objects
On Wed, Apr 27, 2011 at 11:12 PM, Jesse Barnes wrote:
> On Wed, 27 Apr 2011 14:19:05 +0200
> Daniel Vetter wrote:
>
>> Hi Jesse,
>>
>> I like it. It's a bit of a chicken-egg api design problem, but if a
>> proof-of-concept
>> implementation exists for an embedded chip plus something to check whether
>> it's good enough to implement Xv on ancient hw (intel overlay or for the qemu
>> kms driver, if that could do this), that should be good enough.
>>
>> A few comments on the ioctl interface.
>
> Thanks for checking it out.
>
>> > +/* Overlays blend with or override other bits on the CRTC */
>> > +struct drm_mode_set_overlay {
>> > + __u32 overlay_id;
>> > + __u32 crtc_id;
>> > + __u32 fb_id; /* contains surface format type */
>> > +
>> > + __u32 crtc_x, crtc_y;
>> > + __u32 x, y;
>> > +
>> > + /* FIXME: color key/mask, scaling, z-order, other? */
>> > +};
>>
>> I think scaling is required, you can't implement Xv without that. The
>> problem is some arbitraray
>> hw range restrictions, but returning -EINVAL if they're violated looks okay.
>> So
>>
>> float scale_x, scale_y;
>
> Ok, I'll collect more fields based on this thread and make this
> structure bigger. Still not sure how to handle arbitrary blend and
> z-order restrictions without passing a tree around though...
What we send over the ioctl interface (in vmwgfx) to control the
overlays is this:
/**
* struct drm_vmw_control_stream_arg
*
* @stream_id: Stearm to control
* @enabled: If false all following arguments are ignored.
* @handle: Handle to buffer for getting data from.
* @format: Format of the overlay as understood by the host.
* @width: Width of the overlay.
* @height: Height of the overlay.
* @size: Size of the overlay in bytes.
* @pitch: Array of pitches, the two last are only used for YUV12 formats.
* @offset: Offset from start of dma buffer to overlay.
* @src: Source rect, must be within the defined area above.
* @dst: Destination rect, x and y may be negative.
*
* Argument to the DRM_VMW_CONTROL_STREAM Ioctl.
*/
struct drm_vmw_control_stream_arg {
uint32_t stream_id;
uint32_t enabled;
uint32_t flags;
uint32_t color_key;
uint32_t handle;
uint32_t offset;
int32_t format;
uint32_t size;
uint32_t width;
uint32_t height;
uint32_t pitch[3];
uint32_t pad64;
struct drm_vmw_rect src;
struct drm_vmw_rect dst;
};
The command contains information describing the source "framebuffer"
as well as the data for controlling the overlay. The args are by a
amazing coincidence is almost 1:1 what we also send down to the host.
The biggest change here from what you proposed is that we send two
rects describing from where within the buffer we get the data and to
where it should go. My vote is to do that as well (makes my life
easier at least).
Tho I guess that color_key and flags could be made into a property.
Cheers Jakob.
>
>>
>> should be good enough. For the remaining things (color conversion,
>> blending, ...) I don't think
>> there's any generic way to map hw capabilities (without going insane).
>> I think a bunch of
>> properties (with standard stuff for gamma, color key, ...) would be
>> perfect for that. If we
>> set the defaults such that it Just Works for Xv (after setting the
>> color key, if present), that
>> would be great!
>
> Yeah, properties for those is probably the way to go.
>
>>
>> > +struct drm_mode_get_overlay {
>> > + __u64 format_type_ptr;
>> > + __u32 overlay_id;
>> > +
>> > + __u32 crtc_id;
>> > + __u32 fb_id;
>> > +
>> > + __u32 crtc_x, crtc_y;
>> > + __u32 x, y;
>> > +
>> > + __u32 possible_crtcs;
>> > + __u32 gamma_size;
>> > +
>> > + __u32 count_format_types;
>> > +};
>>
>> Imo the real problem with formats is stride restrictions and other hw
>> restrictions (tiling, ...).
>> ARM/v4l people seem to want that to be in the kernel so that they can
>> e.g. dma decoded
>> video streams directly to the gpu (also for other stuff). Perhaps we
>> want to extend the
>> create_dumb_gem_object ioctl for that use case, but I'm not too
>> convinced that this belongs
>> into the kernel.
>
> I think it's a bit like handling tiling formats; for the most part the
> kernel doesn't care because it's not reading or writing the data, but
> it does need to know the format when programming certain regs. So I
> don't think we can avoid having surface format information passed in
> when creating an fb for an overlay. And if we do that we may as well
> enumerate the types we support when overlay info is fetched to make
> portability for app code a little easier.
>
> Jesse
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Radeon support state in libkms
On Wed, Feb 16, 2011 at 7:40 PM, nobled wrote: > On Tue, Feb 15, 2011 at 9:17 AM, Alexandre Demers > wrote: >> On 11-02-11 06:21 AM, Jakob Bornecrantz wrote: >>> On Wed, Feb 9, 2011 at 7:35 PM, Corbin Simpson >>> wrote: >>>> On Wed, Feb 9, 2011 at 6:49 AM, James Simmons >>>> wrote: >>>>>> I was looking at the radeon support state in libkms and I found out the >>>>>> following patch was proposed back in September, but never commented nor >>>>>> merged. The patch was submitted by nobled. >>>>>> >>>>>> http://lists.freedesktop.org/archives/dri-devel/2010-September/003740.html >>>>>> >>>>>> I applied it on the latest drm git version without any problem and >>>>>> compilation looks fine. Is there a reason preventing it from being >>>>>> merged? >>>>>> >>>>>> If it needs testing, make your suggestion, I'll gladly make my best to >>>>>> test it. >>>>> Good point. Is KMS dead? Anyone? >>>> I was seriously hoping that Dave's dumb buffer API would go upstream >>>> and that libkms would be able to use that generic path for all KMS >>>> drivers, rather than needing shim code for every driver it supports. >>>> >>>> That said, yeah, the libkms maintainer probably should pull that code >>>> in. Who is that, anyway? >>>> >>>> ~ C. >>> That would probably be me. >>> >>> The "core" libkms changes are rb/acked and the radeon changes looks >>> good so just go ahead and commit them anyways. Nobled do you have >>> commit access to the drm repo? >>> >>> Cheers Jakob. >> >> I don't think Nobled has commit access. But to be sure, I'm adding it to >> the conversation. ;) >> >> -- >> Alexandre Demers >> >> >> >> > > Haha, no, no commit access. (not subscribed on dri-devel, either, so > thanks for the CC) Do I have your sob, for the patch? > > But while I'm here, is there any advice anywhere for getting patches > noticed a little... sooner? 'Cause I have a whole bunch of *other* > months-old mesa-dev patches nobody's committed yet. Poke us more, sorry sometimes we are just bussy with work... Cheers Jakob.
Re: Radeon support state in libkms
On Wed, Feb 16, 2011 at 7:40 PM, nobled wrote: > On Tue, Feb 15, 2011 at 9:17 AM, Alexandre Demers > wrote: >> On 11-02-11 06:21 AM, Jakob Bornecrantz wrote: >>> On Wed, Feb 9, 2011 at 7:35 PM, Corbin Simpson >>> wrote: >>>> On Wed, Feb 9, 2011 at 6:49 AM, James Simmons >>>> wrote: >>>>>> I was looking at the radeon support state in libkms and I found out the >>>>>> following patch was proposed back in September, but never commented nor >>>>>> merged. The patch was submitted by nobled. >>>>>> >>>>>> http://lists.freedesktop.org/archives/dri-devel/2010-September/003740.html >>>>>> >>>>>> I applied it on the latest drm git version without any problem and >>>>>> compilation looks fine. Is there a reason preventing it from being >>>>>> merged? >>>>>> >>>>>> If it needs testing, make your suggestion, I'll gladly make my best to >>>>>> test it. >>>>> Good point. Is KMS dead? Anyone? >>>> I was seriously hoping that Dave's dumb buffer API would go upstream >>>> and that libkms would be able to use that generic path for all KMS >>>> drivers, rather than needing shim code for every driver it supports. >>>> >>>> That said, yeah, the libkms maintainer probably should pull that code >>>> in. Who is that, anyway? >>>> >>>> ~ C. >>> That would probably be me. >>> >>> The "core" libkms changes are rb/acked and the radeon changes looks >>> good so just go ahead and commit them anyways. Nobled do you have >>> commit access to the drm repo? >>> >>> Cheers Jakob. >> >> I don't think Nobled has commit access. But to be sure, I'm adding it to >> the conversation. ;) >> >> -- >> Alexandre Demers >> >> >> >> > > Haha, no, no commit access. (not subscribed on dri-devel, either, so > thanks for the CC) Do I have your sob, for the patch? > > But while I'm here, is there any advice anywhere for getting patches > noticed a little... sooner? 'Cause I have a whole bunch of *other* > months-old mesa-dev patches nobody's committed yet. Poke us more, sorry sometimes we are just bussy with work... Cheers Jakob. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Radeon support state in libkms
On Wed, Feb 9, 2011 at 7:35 PM, Corbin Simpson wrote: > On Wed, Feb 9, 2011 at 6:49 AM, James Simmons > wrote: >> >>> I was looking at the radeon support state in libkms and I found out the >>> following patch was proposed back in September, but never commented nor >>> merged. The patch was submitted by nobled. >>> >>> http://lists.freedesktop.org/archives/dri-devel/2010-September/003740.html >>> >>> I applied it on the latest drm git version without any problem and >>> compilation looks fine. Is there a reason preventing it from being merged? >>> >>> If it needs testing, make your suggestion, I'll gladly make my best to >>> test it. >> >> Good point. Is KMS dead? Anyone? > > I was seriously hoping that Dave's dumb buffer API would go upstream > and that libkms would be able to use that generic path for all KMS > drivers, rather than needing shim code for every driver it supports. > > That said, yeah, the libkms maintainer probably should pull that code > in. Who is that, anyway? > > ~ C. That would probably be me. The "core" libkms changes are rb/acked and the radeon changes looks good so just go ahead and commit them anyways. Nobled do you have commit access to the drm repo? Cheers Jakob.
Re: Radeon support state in libkms
On Wed, Feb 9, 2011 at 7:35 PM, Corbin Simpson wrote: > On Wed, Feb 9, 2011 at 6:49 AM, James Simmons wrote: >> >>> I was looking at the radeon support state in libkms and I found out the >>> following patch was proposed back in September, but never commented nor >>> merged. The patch was submitted by nobled. >>> >>> http://lists.freedesktop.org/archives/dri-devel/2010-September/003740.html >>> >>> I applied it on the latest drm git version without any problem and >>> compilation looks fine. Is there a reason preventing it from being merged? >>> >>> If it needs testing, make your suggestion, I'll gladly make my best to >>> test it. >> >> Good point. Is KMS dead? Anyone? > > I was seriously hoping that Dave's dumb buffer API would go upstream > and that libkms would be able to use that generic path for all KMS > drivers, rather than needing shim code for every driver it supports. > > That said, yeah, the libkms maintainer probably should pull that code > in. Who is that, anyway? > > ~ C. That would probably be me. The "core" libkms changes are rb/acked and the radeon changes looks good so just go ahead and commit them anyways. Nobled do you have commit access to the drm repo? Cheers Jakob. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] i915: Fix comments about cube layouts
---
src/mesa/drivers/dri/i915/i915_tex_layout.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i915/i915_tex_layout.c
b/src/mesa/drivers/dri/i915/i915_tex_layout.c
index 7026552..c98dede 100644
--- a/src/mesa/drivers/dri/i915/i915_tex_layout.c
+++ b/src/mesa/drivers/dri/i915/i915_tex_layout.c
@@ -67,7 +67,8 @@ static GLint bottom_offsets[6] = {
/**
- * Cube texture map layout for i830M-GM915.
+ * Cube texture map layout for i830M-GM915 and
+ * none compressed cube texture map on GM945.
*
* Hardware layout looks like:
*
@@ -258,7 +259,7 @@ i915_miptree_layout(struct intel_context *intel, struct
intel_mipmap_tree * mt,
/**
- * Cube texture map layout for GM945 and later.
+ * Compressed cube texture map layout for GM945 and later.
*
* The hardware layout looks like the 830-915 layout, except for the small
* sizes. A zoomed in view of the layout for 945 is:
--
1.6.0.4
[PATCH] i915: Fix comments about cube layouts
---
src/mesa/drivers/dri/i915/i915_tex_layout.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i915/i915_tex_layout.c
b/src/mesa/drivers/dri/i915/i915_tex_layout.c
index 7026552..c98dede 100644
--- a/src/mesa/drivers/dri/i915/i915_tex_layout.c
+++ b/src/mesa/drivers/dri/i915/i915_tex_layout.c
@@ -67,7 +67,8 @@ static GLint bottom_offsets[6] = {
/**
- * Cube texture map layout for i830M-GM915.
+ * Cube texture map layout for i830M-GM915 and
+ * none compressed cube texture map on GM945.
*
* Hardware layout looks like:
*
@@ -258,7 +259,7 @@ i915_miptree_layout(struct intel_context *intel, struct
intel_mipmap_tree * mt,
/**
- * Cube texture map layout for GM945 and later.
+ * Compressed cube texture map layout for GM945 and later.
*
* The hardware layout looks like the 830-915 layout, except for the small
* sizes. A zoomed in view of the layout for 945 is:
--
1.6.0.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
