Re: [Intel-gfx] [PATCH v2 5/5] ACPI: button: Obselete acpi_lid_open() invocations
Hi, Benjamin > From: linux-acpi-ow...@vger.kernel.org > [mailto:linux-acpi-ow...@vger.kernel.org] On Behalf Of Benjamin > Tissoires > Subject: Re: [PATCH v2 5/5] ACPI: button: Obselete acpi_lid_open() invocations > > Hi Lv, > > I am trying to reduce the number of parallel discussion we have on the > same subject, but there is something here I can't let you have. OK. So let's stop talking in PATCH 4-5. They are actually cleanups from my point of view. In fact, I don't see any big conflicts between us. My point of view is: There is a gap between (BIOS ensured/Windows expected) acpi control method lid device behavior and Linux user space expected acpi control method lid device behavior. Button driver default behavior should be: button.lid_init_state=ignore If user space programs have special needs, they can fix problems on their own, via the following mean: echo -n "open" > /sys/modules/button/parameters/lid_init_state echo -n "close" > /sys/modules/button/parameters/lid_init_state Keeping open/close modes is because I don't think there is any bug in button driver. So I need to prepare quirk modes from button driver's point of view and use them as a response to related bug reports reported in acpi community. Your point of view is: There is a gap between (BIOS ensured/Windows expected) acpi control method lid device behavior and Linux user space expected acpi control method lid device behavior. Button driver default behavior should be (not 100% sure if this is your opinion): button.lid_init_state=method If user space programs have special needs, they can fix them on their own, via the following mean: libinput:name:*Lid Switch*:dmi:*svnMicrosoftCorporation:pnSurface3:* LIBINPUT_ATTR_LID_SWITCH_RELIABILITY=write_open From this point of view, we actually don't need open/close modes. It seems we just need to determine the following first: 1. Who should be responsible for solving bugs triggered by the conflict between bios and linux user space expectations: button driver? libinput? Some other user space programs? Users? 2. What should be the default button driver behavior? button.lid_init_state=ignore? button.lid_init_state=method? 3. If non button driver quirks are working, button driver quirk modes are useless. The question is: Should button.lid_init_state=open/close be kept? 4. From button driver's point of view, button.lid_init_state=ignore seems to be always correct, so we won't abandon it. If we can use libinput to manage platform quirks, then button.lid_init_state=method also looks useless. The question is: Should button.lid_init_state=method be kept? I should also let you know my preference: 1. using button.lid_init_state=ignore and button.lid_init_state=method as default behavior is ok for me if answer to 1 is not button driver, otherwise using button.lid_init_state=method is not ok for me 2. deletion of button.lid_init_state=open/close is ok for me if answer to 1 is not button driver, otherwise deletion of button.lid_init_state=open/close is not ok for me 3. deletion of button.lid_init_state=method is always ok for me See the base line from my side is very clear: If acpi community need to handle such bug reports, button.lid_init_state=method cannot be the default behavior. We are just using a different default behavior than "method" to drive things to reach the final root caused solution. Could you let me know your preference so that we can figure out an agreement between us. Though I don't know if end users will buy it (they may keep on filing regression reports in ACPI community). Can this make the discussion simpler? So before determining whether we should keep button.lid_init_state=open/close or not. We obviously should stop talking here. You can copy above questions to PATCH 1-2 discussion and reply in order to stop this. We can revisit PATCH 4-5 when the basic questions are answered. :) > > On Fri, May 12, 2017 at 8:08 AM, Zheng, Lv wrote: > > Hi, > > > > If my previous reply is not persuasive enough. > > Let me do that in a different way. > > > >> From: linux-acpi-ow...@vger.kernel.org > >> [mailto:linux-acpi-ow...@vger.kernel.org] On Behalf Of Zheng, > >> Lv > >> Subject: RE: [PATCH v2 5/5] ACPI: button: Obselete acpi_lid_open() > >> invocations > >> > >> Hi, > >> > >> > From: Benjamin Tissoires [mailto:benjamin.tissoi...@gmail.com] > >> > Subject: Re: [PATCH v2 5/5] ACPI: button: Obselete acpi_lid_open() > >> > invocations > >> > > >> > On Tue, May 9, 2017 at 9:02 AM, Lv Zheng wrote: > >> > > Since notification side has been changed to always notify kernel > >> > > listeners > >> > > using _LID returning value. Now listeners needn't invoke > >> > > acpi_lid_open(), > >> > > it should use a spec suggested control method lid device usage model: > >> > > register lid notification and use the notified value instead, which is > >> > > the > >> > > only way to ensure the value is correct for > >> > > "button.lid_init_state=ignore" > >>
Re: [Intel-gfx] [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
Hi Alex and Gerd, >-Original Message- >From: Alex Williamson [mailto:alex.william...@redhat.com] >Sent: Saturday, May 13, 2017 12:38 AM >To: Gerd Hoffmann >Cc: Chen, Xiaoguang ; Tian, Kevin >; intel-gfx@lists.freedesktop.org; linux- >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan >; intel-gvt-...@lists.freedesktop.org; Wang, Zhi A > >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf > >On Fri, 12 May 2017 11:12:05 +0200 >Gerd Hoffmann wrote: > >> Hi, >> >> > If the contents of the framebuffer change or if the parameters of >> > the framebuffer change? I can't image that creating a new dmabuf fd >> > for every visual change within the framebuffer would be efficient, >> > but I don't have any concept of what a dmabuf actually does. >> >> Ok, some background: >> >> The drm subsystem has the concept of planes. The most important plane >> is the primary framebuffer (i.e. what gets scanned out to the physical >> display). The cursor is a plane too, and there can be additional >> overlay planes for stuff like video playback. >> >> Typically there are multiple planes in a system and only one of them >> gets scanned out to the crtc, i.e. the fbdev emulation creates one >> plane for the framebuffer console. The X-Server creates a plane too, >> and when you switch between X-Server and framebuffer console via >> ctrl-alt-fn the intel driver just reprograms the encoder to scan out >> the one or the other plane to the crtc. >> >> The dma-buf handed out by gvt is a reference to a plane. I think on >> the host side gvt can see only the active plane (from encoder/crtc >> register >> programming) not the inactive ones. >> >> The dma-buf can be imported as opengl texture and then be used to >> render the guest display to a host window. I think it is even >> possible to use the dma-buf as plane in the host drm driver and scan >> it out directly to a physical display. The actual framebuffer content >> stays in gpu memory all the time, the cpu never has to touch it. >> >> It is possible to cache the dma-buf handles, i.e. when the guest boots >> you'll get the first for the fbcon plane, when the x-server starts the >> second for the x-server framebuffer, and when the user switches to the >> text console via ctrl-alt-fn you can re-use the fbcon dma-buf you >> already have. >> >> The caching becomes more important for good performance when the guest >> uses pageflipping (wayland does): define two planes, render into one >> while displaying the other, then flip the two for a atomic display >> update. >> >> The caching also makes it a bit difficult to create a good interface. >> So, the current patch set creates: >> >> (a) A way to query the active planes (ioctl >> INTEL_VGPU_QUERY_DMABUF added by patch 5/6 of this series). >> (b) A way to create a dma-buf for the active plane (ioctl >> INTEL_VGPU_GENERATE_DMABUF). >> >> Typical userspace workflow is to first query the plane, then check if >> it already has a dma-buf for it, and if not create one. > >Thank you! This is immensely helpful! > >> > What changes to the framebuffer require a new dmabuf fd? Shouldn't >> > the user query the parameters of the framebuffer through a dmabuf fd >> > and shouldn't the dmabuf fd have some signaling mechanism to the >> > user (eventfd perhaps) to notify the user to re-evaluate the parameters? >> >> dma-bufs don't support that, they are really just a handle to a piece >> of memory, all metadata (format, size) most be communicated by other means. >> >> > Otherwise are you imagining that the user polls the vfio region? >> >> Hmm, notification support would probably a good reason to have a >> separate file handle to manage the dma-bufs (instead of using >> driver-specific ioctls on the vfio fd), because the driver could also >> use the management fd for notifications then. > >I like this idea of a separate control fd for dmabufs, it provides not only a >central >management point, but also a nice abstraction for the vfio device specific >interface. We potentially only need a single >VFIO_DEVICE_GET_DMABUF_MGR_FD() ioctl to get a dmabuf management fd >(perhaps with a type parameter, ex. GFX) where maybe we could have vfio-core >incorporate this reference into the group lifecycle, so the vendor driver only >needs to fdget/put this manager fd for the various plane dmabuf fds spawned in >order to get core-level reference counting. Following is my understanding of the management fd idea: 1) QEMU will call VFIO_DEVICE_GET_DMABUF_MGR_FD() ioctl to create a fd and saved the fd in vfio group while initializing the vfio. 2) vendor driver use fdget to add reference count of the fd. 3) vendor driver use ioctl to the fd to query plane information or create dma-buf fd. 4) vendor driver use fdput when finished using this fd. Is my understanding right? Both QEMU and kernel vfio-core will have changes based on this proposal except the vendor part changes. Who will make these changes? T
[Intel-gfx] [PATCH] drm/i915: fix noderef.cocci warnings
drivers/gpu/drm/i915/i915_syncmap.c:285:45-51: ERROR: application of sizeof to pointer sizeof when applied to a pointer typed expression gives the size of the pointer Generated by: scripts/coccinelle/misc/noderef.cocci Signed-off-by: Fengguang Wu --- i915_syncmap.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/gpu/drm/i915/i915_syncmap.c +++ b/drivers/gpu/drm/i915/i915_syncmap.c @@ -282,7 +282,7 @@ static noinline int __sync_set(struct i9 unsigned int above; /* Insert a join above the current layer */ - next = kzalloc(sizeof(*next) + KSYNCMAP * sizeof(next), + next = kzalloc(sizeof(*next) + KSYNCMAP * sizeof(*next), GFP_KERNEL); if (unlikely(!next)) return -ENOMEM; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [drm-intel:drm-intel-next-queued 4/7] drivers/gpu/drm/i915/i915_syncmap.c:285:45-51: ERROR: application of sizeof to pointer
tree: git://anongit.freedesktop.org/drm-intel drm-intel-next-queued head: 49f08598bf7a52eadebda851a5e8e6fa1dc2e15e commit: 4797948071f607c5b43753cb8f1b7ddcf22e146d [4/7] drm/i915: Squash repeated awaits on the same fence coccinelle warnings: (new ones prefixed by >>) >> drivers/gpu/drm/i915/i915_syncmap.c:285:45-51: ERROR: application of sizeof >> to pointer Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/dp: start a DPCD based DP sink/branch device quirk database
On Fri, May 12, 2017 at 9:57 AM, Jani Nikula wrote: > Personally I don't think enums should be used for defining bits, because > they are not enumerations. The bits are usually combined to come up with > values that are not part of the enum. > > If we added the quirks to struct intel_dp_desc (and moved to drm > helpers) we could use enums for defining the bit shifts and add a > function to query for a specific quirk, say, drm_dp_has_quirk(struct > drm_dp_desc *desc, enum drm_dpcd_quirk quirk). It's a bit a gnu-ism afaik, but enums-as-bitfields is very much standard. gdb even decodes it for you into the individual bits iirc. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
