[PATCH 2/2 v6] of: add generic videomode description
Hi Steffen, On Monday 08 October 2012 14:48:01 Steffen Trumtrar wrote: > On Mon, Oct 08, 2012 at 02:13:50PM +0200, Laurent Pinchart wrote: > > On Thursday 04 October 2012 19:59:20 Steffen Trumtrar wrote: > > > Get videomode from devicetree in a format appropriate for the > > > backend. drm_display_mode and fb_videomode are supported atm. > > > Uses the display signal timings from of_display_timings > > > > > > Signed-off-by: Steffen Trumtrar > > > --- > > > > > > drivers/of/Kconfig |5 + > > > drivers/of/Makefile |1 + > > > drivers/of/of_videomode.c| 212 +++ > > > include/linux/of_videomode.h | 41 > > > 4 files changed, 259 insertions(+) > > > create mode 100644 drivers/of/of_videomode.c > > > create mode 100644 include/linux/of_videomode.h [snip] > > > diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c > > > new file mode 100644 > > > index 000..76ac16e > > > --- /dev/null > > > +++ b/drivers/of/of_videomode.c [snip] > > > +int videomode_from_timing(struct display_timings *disp, struct > > > videomode *vm, > > > + int index) > > > +{ > > > + struct signal_timing *st = NULL; > > > + > > > + if (!vm) > > > + return -EINVAL; > > > + > > > > What about making vm a mandatory argument ? It looks to me like a caller > > bug if vm is NULL. > > The caller must provide the struct videomode, yes. Wouldn't the kernel hang > itself with a NULL pointer exception, if I just work with it ? The kernel would oops, clearly showing the caller that a non-null vm is needed :-) > > > + st = display_timings_get(disp, index); > > > + > > > > You can remove the blank line. > > > > > + if (!st) { > > > + pr_err("%s: no signal timings found\n", __func__); > > > + return -EINVAL; > > > + } > > > + > > > + vm->pixelclock = signal_timing_get_value(>pixelclock, 0); > > > + vm->hactive = signal_timing_get_value(>hactive, 0); > > > + vm->hfront_porch = signal_timing_get_value(>hfront_porch, 0); > > > + vm->hback_porch = signal_timing_get_value(>hback_porch, 0); > > > + vm->hsync_len = signal_timing_get_value(>hsync_len, 0); > > > + > > > + vm->vactive = signal_timing_get_value(>vactive, 0); > > > + vm->vfront_porch = signal_timing_get_value(>vfront_porch, 0); > > > + vm->vback_porch = signal_timing_get_value(>vback_porch, 0); > > > + vm->vsync_len = signal_timing_get_value(>vsync_len, 0); > > > + > > > + vm->vah = st->vsync_pol_active_high; > > > + vm->hah = st->hsync_pol_active_high; > > > + vm->interlaced = st->interlaced; > > > + vm->doublescan = st->doublescan; > > > + > > > + return 0; > > > +} > > > + > > > +int of_get_videomode(struct device_node *np, struct videomode *vm, int > > > index) > > > > I wonder how to avoid abuse of this functions. It's a useful helper for > > drivers that need to get a video mode once only, but would result in lower > > performances if a driver calls it for every mode. Drivers must call > > of_get_display_timing_list instead in that case and case the display > > timings. I'm wondering whether we should really expose of_get_videomode. > > The intent was to let the driver decide. That way all the other overhead may > be skipped. My point is that driver writers might just call of_get_videomode() in a loop, not knowing that it's expensive. I want to avoid that. We need to at least add kerneldoc to the function stating that this shouldn't be done. > > > +{ > > > + struct display_timings *disp; > > > + int ret = 0; > > > > No need to assign ret to 0 here. > > Ah, yes. Unneeded in this case. > > > > + > > > + disp = of_get_display_timing_list(np); > > > + > > > > You can remove the blank line. > > > > > + if (!disp) { > > > + pr_err("%s: no timings specified\n", __func__); > > > + return -EINVAL; > > > + } > > > + > > > + if (index == OF_DEFAULT_TIMING) > > > + index = disp->default_timing; > > > + > > > + ret = videomode_from_timing(disp, vm, index); > > > + > > > > No need for a blank line. > > > > > + if (ret) > > > + return ret; > > > + > > > + display_timings_release(disp); > > > + > > > + if (!vm) { > > > + pr_err("%s: could not get videomode %d\n", __func__, index); > > > + return -EINVAL; > > > + } > > > > This can't happen. If vm is NULL the videomode_from_timing call above will > > return -EINVAL, and this function will then return immediately without > > reaching this code block. > > Okay. > > > > + > > > + return 0; > > > +} > > > +EXPORT_SYMBOL_GPL(of_get_videomode); -- Regards, Laurent Pinchart
[PATCH v2] gma500: medfield: drop bogus NULL check in mdfld_dsi_output_init()
From: Wei YongjunDrop the NULL test for dev since it never be NULL. dpatch engine is used to auto generate this patch. (https://github.com/weiyj/dpatch) Signed-off-by: Wei Yongjun --- drivers/gpu/drm/gma500/mdfld_dsi_output.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_output.c b/drivers/gpu/drm/gma500/mdfld_dsi_output.c index 32dba2a..b6f49d9 100644 --- a/drivers/gpu/drm/gma500/mdfld_dsi_output.c +++ b/drivers/gpu/drm/gma500/mdfld_dsi_output.c @@ -506,7 +506,7 @@ void mdfld_dsi_output_init(struct drm_device *dev, dev_dbg(dev->dev, "init DSI output on pipe %d\n", pipe); - if (!dev || ((pipe != 0) && (pipe != 2))) { + if ((pipe != 0) && (pipe != 2)) { DRM_ERROR("Invalid parameter\n"); return; }
[Bug 53544] [915GM] Incorrect modeline due to incorrect EDID block for LG SL80 TV
https://bugs.freedesktop.org/show_bug.cgi?id=53544 --- Comment #14 from Paul Menzel --- In #intel-gfx, ajax confirmed that this is probably a bug in the 915(GM) part of the driver. Unfortunately Intel has not yet released the gen3 documentation, so for non-Intel employees it will be hard to fix this issue. Comparing register dumps of a working and non-working Linux kernel would be one option, but unfortunately as seen in bug 26294 I could not find a working one, although I think it is a regression. In the meantime ajax asked me if vesa works (boot with `nomodeset`) and I will try to get back with this information as soon as possible. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121008/e62eb6a6/attachment-0001.html>
[Bug 55557] Regnum Online UBO break after game update
https://bugs.freedesktop.org/show_bug.cgi?id=7 --- Comment #3 from Rafael Castillo --- Created attachment 68289 --> https://bugs.freedesktop.org/attachment.cgi?id=68289=edit Shader dump requested attached shader dump -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121008/a61af012/attachment.html>
Update on the CEC API
On Monday 08 October 2012 17:49:00 Hans Verkuil wrote: > On Mon October 8 2012 17:06:20 Florian Fainelli wrote: > > Hi Hans, > > > > On Thursday 27 September 2012 16:33:30 Hans Verkuil wrote: > > > Hi all, > > > > > > During the Linux Plumbers Conference we (i.e. V4L2 and DRM developers) > > > had a > > > discussion on how to handle the CEC protocol that's part of the HDMI > > standard. > > > > > > The decision was made to create a CEC bus with CEC clients, each > > > represented > > > by a /dev/cecX device. So this is independent of the V4L or DRM APIs. > > > > > > In addition interested subsystems (alsa for the Audio Return Channel, and > > > possibly networking as well for ethernet over HDMI) can intercept/send CEC > > > messages as well if needed. Particularly for the CEC additions made in > > > HDMI 1.4a it is no longer possible to handle the CEC protocol completely > > > in > > > userspace, but part of the intelligence has to be moved to kernelspace. > > > > What kind of "intelligence" are you talking about? I see nothing in HDMI > > 1.4a > > or earlier that requires doing stuff in kernelspace besides managing > > control to > > the hardware, but I might be missing something. > > Most notably: handling the new hotplug message. That's something that kernel > drivers need to know. Some ARC messages might be relevant for ALSA drivers as > well, but I need to look into those more carefully. > > Also remote control messages might optionally be handled through an input > driver. Ok, then maybe just stick to the standard CEC_UI_* key codes, and let people having proprietary UI functions do the rest in user-space, or write their own input driver. > > > In my opinion ARC is just a control mechanism, and can be dealt with in > > user- > > space, since you really want to just have hints about when ARC is > > enabled/disabled to take appropriate actions on the audio outputs or your > > system. > > > > > > > > I've started working on this API but I am still at the stage of playing > > > around with it and thinking about the best way this functionality should > > > be exposed. At least I managed to get the first CEC packets transferred > > > today :-) > > > > > > It will probably be a few weeks before I post something, but in the > > > meantime > > > if you want to use CEC and have certain requirements that need to be met, > > > please let me know. If only so that I can be certain I haven't forgotten > > > anything. > > > > Here is my wish-list, if I may: > > - allow for a CEC adapter to be in "detached" / "attached" mode, > > particularly > > useful if the hardware doing CEC can process a basic set of messages to act > > a > > a global wake-up source for the system > > I have hardware that can do that, so I want to look into supporting this. > > > - allow for a CEC adapter to define several receive modes: unicast and > > "promiscuous", which is useful for dumping the CEC bus messages > > I don't think I have hardware for that, but it shouldn't be difficult to add. Definitively not, just let a driver writer specify a flag to advertise this capability and have a getter/setter to specify the receive mode. > > > - make the CEC adapter API asynchronous for the data path, so it is easy > > for a > > driver to report completion of a successfully transmitted/received CEC frame > > Already done. Great, thanks! -- Florian
[PATCH 1/2 v6] of: add helper to parse display timings
Hi Stephen, On Monday 08 October 2012 10:10:31 Stephen Warren wrote: > On 10/08/2012 02:25 AM, Guennadi Liakhovetski wrote: > > On Fri, 5 Oct 2012, Stephen Warren wrote: > >> On 10/04/2012 03:35 PM, Guennadi Liakhovetski wrote: > >>> Hi Steffen > >>> > >>> Sorry for chiming in so late in the game, but I've long been wanting to > >>> have a look at this and compare with what we do for V4L2, so, this seems > >>> a great opportunity to me:-) > >>> > >>> On Thu, 4 Oct 2012, Steffen Trumtrar wrote: > diff --git > a/Documentation/devicetree/bindings/video/display-timings.txt > b/Documentation/devicetree/bindings/video/display-timings.txt > > +timings-subnode > +--- > + > +required properties: > + - hactive, vactive: Display resolution > + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing > parameters + in pixels > + vfront-porch, vback-porch, vsync-len: Vertical display timing > parameters in + lines > + - clock: displayclock in Hz > >>> > >>> You're going to hate me for this, but eventually we want to actually > >>> reference clock objects in our DT bindings. For now, even if you don't > >>> want to actually add clock phandles and stuff here, I think, using the > >>> standard "clock-frequency" property would be much better! > >> > >> In a definition of a display timing, we will never need to use the clock > >> binding; the clock binding would be used by the HW module that is > >> generating a timing, not by the timing definition itself. > > > > You mean clock consumer bindings will be in the display device DT node? > > And the display-timings node will be its child? > > Yes > > ... > > + - interlaced (bool) > >>> > >>> Is "interlaced" a property of the hardware, i.e. of the board? Can the > >>> same display controller on one board require interlaced data and on > >>> another board - progressive? > >> > >> Interlace is a property of a display mode. It's quite possible for a > >> particular display controller to switch between interlace and > >> progressive output at run-time. For example, reconfiguring the output > >> between 480i, 720p, 1080i, 1080p modes. Admittedly, if you're talking to > >> a built-in LCD display, you're probably always going to be driving the > >> single mode required by the panel, and that mode will likely always be > >> progressive. However, since this binding attempts to describe any > >> display timing, I think we still need this property per mode. > > > > But why do you need this in the DT then at all? > > Because the driver for the display controller has no idea what display > or panel will be connected to it. That's right, but with the current common panel framework (WIP, I plan to resume working on it this week) the display controller will be able to query the panel for bus configuration parameters (or the other way around, the panel driver will set the display controller bus configuration). The panel driver will still need to receive timing information from DT, but it will be possible to communicate display bus configuration parameters between the panel driver and the display controller driver. This being said, I agree that interlaced is a property of the display mode, not of the display bus (even though the display bus will then be configured for interlaced data). > > If it's fixed, as required per display controller, then its driver will > > know it. If it's runtime configurable, then it's a purely software > > parameter and doesn't depend on the board? > > interlace-vs-progressive isn't "fixed, as required per display > controller", but is a property of the mode being sent by the display > controller, and the requirements for that mode are driven by the > panel/display connected to the display controller, not the display > controller, in general. > > ... > > >>> BTW, I'm not very familiar with display interfaces, but for interlaced > >>> you probably sometimes use a field signal, whose polarity you also want > >>> to specify here? We use a "field-even-active" integer property for it. > >> > >> I think that's a property of the display controller itself, rather than > >> an individual mode, although I'm not 100% certain. My assertion is that > >> the physical interface that the display controller is driving will > >> determine whether embedded or separate sync is used, and in the separate > >> sync case, how the field signal is defined, and that all interlace modes > >> driven over that interface will use the same field signal definition. > > > > In general, I might be misunderstanding something, but don't we have to > > distinguish between 2 types of information about display timings: (1) is > > defined by the display controller requirements, is known to the display > > driver and doesn't need to be present in timings DT. We did have some of > > these parameters in board data previously, because we didn't have proper > > display controller
[Bug 55557] Regnum Online UBO break after game update
https://bugs.freedesktop.org/show_bug.cgi?id=7 --- Comment #2 from Rafael Castillo --- ok, today when i get home i'll get a shader dump and post it thanks for your help -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121008/c5f5c1e8/attachment.html>
[PATCH 01/11] drm: add drm_send_vblank_event() helper (v2)
From: Rob ClarkA helper that drivers can use to send vblank event after a pageflip. If the driver doesn't support proper vblank irq based time/seqn then just pass -1 for the pipe # to get do_gettimestamp() behavior (since there are a lot of drivers that don't use drm_vblank_count_and_time()) Also an internal send_vblank_event() helper for the various other code paths within drm_irq that also need to send vblank events. v1: original v2: add back 'vblwait->reply.sequence = seq' which should not have been deleted Signed-off-by: Rob Clark --- drivers/gpu/drm/drm_irq.c | 64 + include/drm/drmP.h|2 ++ 2 files changed, 44 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 076c4a8..0e6e804 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -802,6 +802,43 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, int crtc, } EXPORT_SYMBOL(drm_vblank_count_and_time); +static void send_vblank_event(struct drm_pending_vblank_event *e, + unsigned long seq, struct timeval *now) +{ + e->event.sequence = seq; + e->event.tv_sec = now->tv_sec; + e->event.tv_usec = now->tv_usec; + + list_add_tail(>base.link, + >base.file_priv->event_list); + wake_up_interruptible(>base.file_priv->event_wait); + trace_drm_vblank_event_delivered(e->base.pid, e->pipe, +e->event.sequence); +} + +/** + * drm_send_vblank_event - helper to send vblank event after pageflip + * @dev: DRM device + * @crtc: CRTC in question + * @e: the event to send + * + * Updates sequence # and timestamp on event, and sends it to userspace. + */ +void drm_send_vblank_event(struct drm_device *dev, int crtc, + struct drm_pending_vblank_event *e) +{ + struct timeval now; + unsigned int seq; + if (crtc >= 0) { + seq = drm_vblank_count_and_time(dev, crtc, ); + } else { + seq = 0; + do_gettimeofday(); + } + send_vblank_event(e, seq, ); +} +EXPORT_SYMBOL(drm_send_vblank_event); + /** * drm_update_vblank_count - update the master vblank counter * @dev: DRM device @@ -955,15 +992,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc) DRM_DEBUG("Sending premature vblank event on disable: \ wanted %d, current %d\n", e->event.sequence, seq); - - e->event.sequence = seq; - e->event.tv_sec = now.tv_sec; - e->event.tv_usec = now.tv_usec; + list_del(>base.link); drm_vblank_put(dev, e->pipe); - list_move_tail(>base.link, >base.file_priv->event_list); - wake_up_interruptible(>base.file_priv->event_wait); - trace_drm_vblank_event_delivered(e->base.pid, e->pipe, -e->event.sequence); + send_vblank_event(e, seq, ); } spin_unlock_irqrestore(>vbl_lock, irqflags); @@ -1107,15 +1138,9 @@ static int drm_queue_vblank_event(struct drm_device *dev, int pipe, e->event.sequence = vblwait->request.sequence; if ((seq - vblwait->request.sequence) <= (1 << 23)) { - e->event.sequence = seq; - e->event.tv_sec = now.tv_sec; - e->event.tv_usec = now.tv_usec; drm_vblank_put(dev, pipe); - list_add_tail(>base.link, >base.file_priv->event_list); - wake_up_interruptible(>base.file_priv->event_wait); + send_vblank_event(e, seq, ); vblwait->reply.sequence = seq; - trace_drm_vblank_event_delivered(current->pid, pipe, -vblwait->request.sequence); } else { /* drm_handle_vblank_events will call drm_vblank_put */ list_add_tail(>base.link, >vblank_event_list); @@ -1256,14 +1281,9 @@ static void drm_handle_vblank_events(struct drm_device *dev, int crtc) DRM_DEBUG("vblank event on %d, current %d\n", e->event.sequence, seq); - e->event.sequence = seq; - e->event.tv_sec = now.tv_sec; - e->event.tv_usec = now.tv_usec; + list_del(>base.link); drm_vblank_put(dev, e->pipe); - list_move_tail(>base.link, >base.file_priv->event_list); - wake_up_interruptible(>base.file_priv->event_wait); - trace_drm_vblank_event_delivered(e->base.pid, e->pipe, -e->event.sequence); + send_vblank_event(e, seq, ); } spin_unlock_irqrestore(>event_lock, flags); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 05af0e7..ee8f927 100644 ---
[Bug 55557] Regnum Online UBO break after game update
https://bugs.freedesktop.org/show_bug.cgi?id=7 Ian Romanick changed: What|Removed |Added Status|NEW |NEEDINFO CC||idr at freedesktop.org --- Comment #1 from Ian Romanick --- It looks like they gave us a shader that wouldn't compile. Can you run with the environment variable MESA_GLSL=dump and attach the output? That will at least let us figure out whether the compilation failure is our fault or theirs. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121008/ab08f725/attachment.html>
[PATCH v5 0/3] Enhanced EDID quirk functionality
On 09/28/2012 02:03 AM, Ian Pilcher wrote: > As promised, this patch series does only the following: > > * Add user-defined EDID quirks -- via sysfs or a module parameter > * Add an EDID quirk flag to disable all HDMI functionality >(InfoFrames) > * Add a quirk to disable HDMI InfoFrames for the LG L246WP > ACK? NACK? I'm on crack? (I'm starting to think that this is some sort of hazing ritual.) -- Ian Pilcher arequipeno at gmail.com "If you're going to shift my paradigm ... at least buy me dinner first."
[PATCH v2] gma500: medfield: drop bogus NULL check in mdfld_dsi_output_init()
On Mon, Oct 08, 2012 at 10:44:06PM +0800, Wei Yongjun wrote: > From: Wei Yongjun > > Drop the NULL test for dev since it never be NULL. > > dpatch engine is used to auto generate this patch. > (https://github.com/weiyj/dpatch) > > Signed-off-by: Wei Yongjun > --- > drivers/gpu/drm/gma500/mdfld_dsi_output.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_output.c > b/drivers/gpu/drm/gma500/mdfld_dsi_output.c > index 32dba2a..b6f49d9 100644 > --- a/drivers/gpu/drm/gma500/mdfld_dsi_output.c > +++ b/drivers/gpu/drm/gma500/mdfld_dsi_output.c > @@ -506,7 +506,7 @@ void mdfld_dsi_output_init(struct drm_device *dev, > > dev_dbg(dev->dev, "init DSI output on pipe %d\n", pipe); > > - if (!dev || ((pipe != 0) && (pipe != 2))) { > + if ((pipe != 0) && (pipe != 2)) { It would be nice to remove redundant parentheses while you're there. Otherwise Acked-by: Kirill A. Shutemov > DRM_ERROR("Invalid parameter\n"); > return; > } > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- Kirill A. Shutemov
[PATCH 3/3] drm/radeon: separate pt alloc from lru add
Make it possible to allocate a persistent page table. Signed-off-by: Christian K?nig --- drivers/gpu/drm/radeon/radeon.h |1 + drivers/gpu/drm/radeon/radeon_cs.c |1 + drivers/gpu/drm/radeon/radeon_gart.c | 20 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 54cf9b5..8c42d54 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -1851,6 +1851,7 @@ void radeon_vm_manager_fini(struct radeon_device *rdev); void radeon_vm_init(struct radeon_device *rdev, struct radeon_vm *vm); void radeon_vm_fini(struct radeon_device *rdev, struct radeon_vm *vm); int radeon_vm_alloc_pt(struct radeon_device *rdev, struct radeon_vm *vm); +void radeon_vm_add_to_lru(struct radeon_device *rdev, struct radeon_vm *vm); struct radeon_fence *radeon_vm_grab_id(struct radeon_device *rdev, struct radeon_vm *vm, int ring); void radeon_vm_fence(struct radeon_device *rdev, diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index d59eb59..023c67d 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -478,6 +478,7 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device *rdev, } out: + radeon_vm_add_to_lru(rdev, vm); mutex_unlock(>mutex); mutex_unlock(>vm_manager.lock); return r; diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index 3c4929b..878a54d 100644 --- a/drivers/gpu/drm/radeon/radeon_gart.c +++ b/drivers/gpu/drm/radeon/radeon_gart.c @@ -617,9 +617,6 @@ int radeon_vm_alloc_pt(struct radeon_device *rdev, struct radeon_vm *vm) } if (vm->page_directory != NULL) { - /* update lru */ - list_del_init(>list); - list_add_tail(>list, >vm_manager.lru_vm); return 0; } @@ -653,11 +650,26 @@ retry: return -ENOMEM; } - list_add_tail(>list, >vm_manager.lru_vm); return 0; } /** + * radeon_vm_add_to_lru - add VMs page table to LRU list + * + * @rdev: radeon_device pointer + * @vm: vm to add to LRU + * + * Add the allocated page table to the LRU list (cayman+). + * + * Global mutex must be locked! + */ +void radeon_vm_add_to_lru(struct radeon_device *rdev, struct radeon_vm *vm) +{ + list_del_init(>list); + list_add_tail(>list, >vm_manager.lru_vm); +} + +/** * radeon_vm_grab_id - allocate the next free VMID * * @rdev: radeon_device pointer -- 1.7.9.5
[PATCH 2/3] drm/radeon: don't add the IB pool to all VMs
We want to use VMs without the IB pool in the future. Signed-off-by: Christian K?nig --- drivers/gpu/drm/radeon/radeon.h |2 +- drivers/gpu/drm/radeon/radeon_gart.c | 23 +++ drivers/gpu/drm/radeon/radeon_kms.c | 11 ++- 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index bc6b56b..54cf9b5 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -1848,7 +1848,7 @@ extern void radeon_ttm_set_active_vram_size(struct radeon_device *rdev, u64 size */ int radeon_vm_manager_init(struct radeon_device *rdev); void radeon_vm_manager_fini(struct radeon_device *rdev); -int radeon_vm_init(struct radeon_device *rdev, struct radeon_vm *vm); +void radeon_vm_init(struct radeon_device *rdev, struct radeon_vm *vm); void radeon_vm_fini(struct radeon_device *rdev, struct radeon_vm *vm); int radeon_vm_alloc_pt(struct radeon_device *rdev, struct radeon_vm *vm); struct radeon_fence *radeon_vm_grab_id(struct radeon_device *rdev, diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index b36b615..3c4929b 100644 --- a/drivers/gpu/drm/radeon/radeon_gart.c +++ b/drivers/gpu/drm/radeon/radeon_gart.c @@ -602,7 +602,6 @@ int radeon_vm_evict(struct radeon_device *rdev, struct radeon_vm *vm) * @vm: vm to bind * * Allocate a page table for the requested vm (cayman+). - * Also starts to populate the page table. * Returns 0 for success, error for failure. * * Global and local mutex must be locked! @@ -655,8 +654,7 @@ retry: } list_add_tail(>list, >vm_manager.lru_vm); - return radeon_vm_bo_update_pte(rdev, vm, rdev->ring_tmp_bo.bo, - >ring_tmp_bo.bo->tbo.mem); + return 0; } /** @@ -1241,30 +1239,15 @@ void radeon_vm_bo_invalidate(struct radeon_device *rdev, * @rdev: radeon_device pointer * @vm: requested vm * - * Init @vm (cayman+). - * Map the IB pool and any other shared objects into the VM - * by default as it's used by all VMs. - * Returns 0 for success, error for failure. + * Init @vm fields (cayman+). */ -int radeon_vm_init(struct radeon_device *rdev, struct radeon_vm *vm) +void radeon_vm_init(struct radeon_device *rdev, struct radeon_vm *vm) { - struct radeon_bo_va *bo_va; - int r; - vm->id = 0; vm->fence = NULL; mutex_init(>mutex); INIT_LIST_HEAD(>list); INIT_LIST_HEAD(>va); - - /* map the ib pool buffer at 0 in virtual address space, set -* read only -*/ - bo_va = radeon_vm_bo_add(rdev, vm, rdev->ring_tmp_bo.bo); - r = radeon_vm_bo_set_addr(rdev, bo_va, RADEON_VA_IB_OFFSET, - RADEON_VM_PAGE_READABLE | - RADEON_VM_PAGE_SNOOPED); - return r; } /** diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c index cb8e94d..065980f 100644 --- a/drivers/gpu/drm/radeon/radeon_kms.c +++ b/drivers/gpu/drm/radeon/radeon_kms.c @@ -420,6 +420,7 @@ int radeon_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv) /* new gpu have virtual address space support */ if (rdev->family >= CHIP_CAYMAN) { struct radeon_fpriv *fpriv; + struct radeon_bo_va *bo_va; int r; fpriv = kzalloc(sizeof(*fpriv), GFP_KERNEL); @@ -427,7 +428,15 @@ int radeon_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv) return -ENOMEM; } - r = radeon_vm_init(rdev, >vm); + radeon_vm_init(rdev, >vm); + + /* map the ib pool buffer read only into +* virtual address space */ + bo_va = radeon_vm_bo_add(rdev, >vm, +rdev->ring_tmp_bo.bo); + r = radeon_vm_bo_set_addr(rdev, bo_va, RADEON_VA_IB_OFFSET, + RADEON_VM_PAGE_READABLE | + RADEON_VM_PAGE_SNOOPED); if (r) { radeon_vm_fini(rdev, >vm); kfree(fpriv); -- 1.7.9.5
[PATCH 1/3] drm/radeon: allocate page tables on demand v4
Based on Dmitries work, but splitting the code into page directory and page table handling makes it far more readable and (hopefully) more reliable. Allocations of page tables are made from the SA on demand, that should still work fine since all page tables are of the same size. Also using the fact that allocations from the SA are mostly continuously (except for end of buffer wraps and under very high memory pressure) to group updates send to the chipset specific code into larger chunks. v3: mostly a rewrite of Dmitries previous patch. v4: fix some typos and coding style Signed-off-by: Dmitry Cherkasov Signed-off-by: Christian K?nig Tested-by: Michel D?nzer --- drivers/gpu/drm/radeon/ni.c |2 +- drivers/gpu/drm/radeon/radeon.h | 11 +- drivers/gpu/drm/radeon/radeon_gart.c | 322 ++ 3 files changed, 262 insertions(+), 73 deletions(-) diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c index 9a46f7d..48e2337 100644 --- a/drivers/gpu/drm/radeon/ni.c +++ b/drivers/gpu/drm/radeon/ni.c @@ -1576,7 +1576,7 @@ void cayman_vm_flush(struct radeon_device *rdev, int ridx, struct radeon_vm *vm) radeon_ring_write(ring, 0); radeon_ring_write(ring, PACKET0(VM_CONTEXT0_PAGE_TABLE_END_ADDR + (vm->id << 2), 0)); - radeon_ring_write(ring, vm->last_pfn); + radeon_ring_write(ring, rdev->vm_manager.max_pfn); radeon_ring_write(ring, PACKET0(VM_CONTEXT0_PAGE_TABLE_BASE_ADDR + (vm->id << 2), 0)); radeon_ring_write(ring, vm->pd_gpu_addr >> 12); diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index b04c064..bc6b56b 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -663,9 +663,14 @@ struct radeon_vm { struct list_headlist; struct list_headva; unsignedid; - unsignedlast_pfn; - u64 pd_gpu_addr; - struct radeon_sa_bo *sa_bo; + + /* contains the page directory */ + struct radeon_sa_bo *page_directory; + uint64_tpd_gpu_addr; + + /* array of page tables, one for each page directory entry */ + struct radeon_sa_bo **page_tables; + struct mutexmutex; /* last fence for cs using this vm */ struct radeon_fence *fence; diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index 753b7ca..b36b615 100644 --- a/drivers/gpu/drm/radeon/radeon_gart.c +++ b/drivers/gpu/drm/radeon/radeon_gart.c @@ -423,6 +423,18 @@ void radeon_gart_fini(struct radeon_device *rdev) */ /** + * radeon_vm_num_pde - return the number of page directory entries + * + * @rdev: radeon_device pointer + * + * Calculate the number of page directory entries (cayman+). + */ +static unsigned radeon_vm_num_pdes(struct radeon_device *rdev) +{ + return rdev->vm_manager.max_pfn >> RADEON_VM_BLOCK_SIZE; +} + +/** * radeon_vm_directory_size - returns the size of the page directory in bytes * * @rdev: radeon_device pointer @@ -431,7 +443,7 @@ void radeon_gart_fini(struct radeon_device *rdev) */ static unsigned radeon_vm_directory_size(struct radeon_device *rdev) { - return (rdev->vm_manager.max_pfn >> RADEON_VM_BLOCK_SIZE) * 8; + return RADEON_GPU_PAGE_ALIGN(radeon_vm_num_pdes(rdev) * 8); } /** @@ -451,11 +463,11 @@ int radeon_vm_manager_init(struct radeon_device *rdev) if (!rdev->vm_manager.enabled) { /* allocate enough for 2 full VM pts */ - size = RADEON_GPU_PAGE_ALIGN(radeon_vm_directory_size(rdev)); - size += RADEON_GPU_PAGE_ALIGN(rdev->vm_manager.max_pfn * 8); + size = radeon_vm_directory_size(rdev); + size += rdev->vm_manager.max_pfn * 8; size *= 2; r = radeon_sa_bo_manager_init(rdev, >vm_manager.sa_manager, - size, + RADEON_GPU_PAGE_ALIGN(size), RADEON_GEM_DOMAIN_VRAM); if (r) { dev_err(rdev->dev, "failed to allocate vm bo (%dKB)\n", @@ -476,7 +488,7 @@ int radeon_vm_manager_init(struct radeon_device *rdev) /* restore page table */ list_for_each_entry(vm, >vm_manager.lru_vm, list) { - if (vm->sa_bo == NULL) + if (vm->page_directory == NULL) continue; list_for_each_entry(bo_va, >va, vm_list) { @@ -500,16 +512,25 @@ static void radeon_vm_free_pt(struct radeon_device *rdev, struct radeon_vm *vm) { struct radeon_bo_va *bo_va; + int i; - if (!vm->sa_bo) + if (!vm->page_directory) return;
Update on the CEC API
On Mon October 8 2012 17:06:20 Florian Fainelli wrote: > Hi Hans, > > On Thursday 27 September 2012 16:33:30 Hans Verkuil wrote: > > Hi all, > > > > During the Linux Plumbers Conference we (i.e. V4L2 and DRM developers) had a > > discussion on how to handle the CEC protocol that's part of the HDMI > standard. > > > > The decision was made to create a CEC bus with CEC clients, each represented > > by a /dev/cecX device. So this is independent of the V4L or DRM APIs. > > > > In addition interested subsystems (alsa for the Audio Return Channel, and > > possibly networking as well for ethernet over HDMI) can intercept/send CEC > > messages as well if needed. Particularly for the CEC additions made in > > HDMI 1.4a it is no longer possible to handle the CEC protocol completely in > > userspace, but part of the intelligence has to be moved to kernelspace. > > What kind of "intelligence" are you talking about? I see nothing in HDMI 1.4a > or earlier that requires doing stuff in kernelspace besides managing control > to > the hardware, but I might be missing something. Most notably: handling the new hotplug message. That's something that kernel drivers need to know. Some ARC messages might be relevant for ALSA drivers as well, but I need to look into those more carefully. Also remote control messages might optionally be handled through an input driver. > In my opinion ARC is just a control mechanism, and can be dealt with in user- > space, since you really want to just have hints about when ARC is > enabled/disabled to take appropriate actions on the audio outputs or your > system. > > > > > I've started working on this API but I am still at the stage of playing > > around with it and thinking about the best way this functionality should > > be exposed. At least I managed to get the first CEC packets transferred > > today :-) > > > > It will probably be a few weeks before I post something, but in the meantime > > if you want to use CEC and have certain requirements that need to be met, > > please let me know. If only so that I can be certain I haven't forgotten > > anything. > > Here is my wish-list, if I may: > - allow for a CEC adapter to be in "detached" / "attached" mode, particularly > useful if the hardware doing CEC can process a basic set of messages to act a > a global wake-up source for the system I have hardware that can do that, so I want to look into supporting this. > - allow for a CEC adapter to define several receive modes: unicast and > "promiscuous", which is useful for dumping the CEC bus messages I don't think I have hardware for that, but it shouldn't be difficult to add. > - make the CEC adapter API asynchronous for the data path, so it is easy for > a > driver to report completion of a successfully transmitted/received CEC frame Already done. Regards, Hans
[PATCH] drm/radeon: check if pcie gen 2 is already enabled
From: Alex DeucherIf so, skip enabling it. Signed-off-by: Alex Deucher --- drivers/gpu/drm/radeon/evergreen.c |8 ++-- drivers/gpu/drm/radeon/r600.c |7 ++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index a1f49c5..888c798 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -3431,9 +3431,13 @@ void evergreen_pcie_gen2_enable(struct radeon_device *rdev) if (!(mask & DRM_PCIE_SPEED_50)) return; - DRM_INFO("enabling PCIE gen 2 link speeds, disable with radeon.pcie_gen2=0\n"); - speed_cntl = RREG32_PCIE_P(PCIE_LC_SPEED_CNTL); + if (speed_cntl & LC_CURRENT_DATA_RATE) { + DRM_INFO("PCIE gen 2 link speeds already enabled\n"); + return; + } else + DRM_INFO("enabling PCIE gen 2 link speeds, disable with radeon.pcie_gen2=0\n"); + if ((speed_cntl & LC_OTHER_SIDE_EVER_SENT_GEN2) || (speed_cntl & LC_OTHER_SIDE_SUPPORTS_GEN2)) { diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index 70c800f..20e4fad 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -3703,7 +3703,12 @@ static void r600_pcie_gen2_enable(struct radeon_device *rdev) if (!(mask & DRM_PCIE_SPEED_50)) return; - DRM_INFO("enabling PCIE gen 2 link speeds, disable with radeon.pcie_gen2=0\n"); + speed_cntl = RREG32_PCIE_P(PCIE_LC_SPEED_CNTL); + if (speed_cntl & LC_CURRENT_DATA_RATE) { + DRM_INFO("PCIE gen 2 link speeds already enabled\n"); + return; + } else + DRM_INFO("enabling PCIE gen 2 link speeds, disable with radeon.pcie_gen2=0\n"); /* 55 nm r6xx asics */ if ((rdev->family == CHIP_RV670) || -- 1.7.7.5
[PATCH] drm/radeon: update comments to clarify VM setup
On 08.10.2012 15:50, alexdeucher at gmail.com wrote: > From: Alex Deucher > > The actual set up and assignment of VM page tables > is done on the fly in radeon_gart.c. > > Signed-off-by: Alex Deucher One comment below, apart from that it's: Reviewed-by: Christian K?nig > --- > drivers/gpu/drm/radeon/ni.c|4 > drivers/gpu/drm/radeon/radeon_device.c |3 +++ > drivers/gpu/drm/radeon/si.c|7 --- > 3 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c > index 48e2337..cfb9276 100644 > --- a/drivers/gpu/drm/radeon/ni.c > +++ b/drivers/gpu/drm/radeon/ni.c > @@ -770,6 +770,10 @@ static int cayman_pcie_gart_enable(struct radeon_device > *rdev) > WREG32(0x15DC, 0); > > /* empty context1-7 */ > + /* Assign the pt base to something valid for now; the pts used for > + * the VMs are determined by the application and setup and assigned > + * on the fly in the vm part of radeon_gart.c > + */ > for (i = 1; i < 8; i++) { > WREG32(VM_CONTEXT0_PAGE_TABLE_START_ADDR + (i << 2), 0); > WREG32(VM_CONTEXT0_PAGE_TABLE_END_ADDR + (i << 2), 0); > diff --git a/drivers/gpu/drm/radeon/radeon_device.c > b/drivers/gpu/drm/radeon/radeon_device.c > index 64a4264..1fad47e 100644 > --- a/drivers/gpu/drm/radeon/radeon_device.c > +++ b/drivers/gpu/drm/radeon/radeon_device.c > @@ -1018,6 +1018,9 @@ int radeon_device_init(struct radeon_device *rdev, > return r; > /* initialize vm here */ > mutex_init(>vm_manager.lock); > + /* FIXME start with 4G, once using 2 level pt switch to full > + * vm size space > + */ Even with 2 level pt I don't think we want to switch to the full vm size space. Having 40bits address space minus 12bits offsets gives us 28bits for the page directory/page table, assuming a 50/50 split between them we would get 2 ^ 14 * 8 = 128kb for each page, which in my eyes is a bit much (and that gets even worth when we get 48bits address space.) I would rather say to make max_pfn depend on the available memory, so that a single application can at least map all VRAM + GART memory. > rdev->vm_manager.max_pfn = 1 << 20; > INIT_LIST_HEAD(>vm_manager.lru_vm); > > diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c > index c76825f..f272ead 100644 > --- a/drivers/gpu/drm/radeon/si.c > +++ b/drivers/gpu/drm/radeon/si.c > @@ -2407,12 +2407,13 @@ static int si_pcie_gart_enable(struct radeon_device > *rdev) > WREG32(0x15DC, 0); > > /* empty context1-15 */ > - /* FIXME start with 4G, once using 2 level pt switch to full > - * vm size space > - */ > /* set vm size, must be a multiple of 4 */ > WREG32(VM_CONTEXT1_PAGE_TABLE_START_ADDR, 0); > WREG32(VM_CONTEXT1_PAGE_TABLE_END_ADDR, rdev->vm_manager.max_pfn); > + /* Assign the pt base to something valid for now; the pts used for > + * the VMs are determined by the application and setup and assigned > + * on the fly in the vm part of radeon_gart.c > + */ > for (i = 1; i < 16; i++) { > if (i < 8) > WREG32(VM_CONTEXT0_PAGE_TABLE_BASE_ADDR + (i << 2),
Update on the CEC API
Hi Hans, On Thursday 27 September 2012 16:33:30 Hans Verkuil wrote: > Hi all, > > During the Linux Plumbers Conference we (i.e. V4L2 and DRM developers) had a > discussion on how to handle the CEC protocol that's part of the HDMI standard. > > The decision was made to create a CEC bus with CEC clients, each represented > by a /dev/cecX device. So this is independent of the V4L or DRM APIs. > > In addition interested subsystems (alsa for the Audio Return Channel, and > possibly networking as well for ethernet over HDMI) can intercept/send CEC > messages as well if needed. Particularly for the CEC additions made in > HDMI 1.4a it is no longer possible to handle the CEC protocol completely in > userspace, but part of the intelligence has to be moved to kernelspace. What kind of "intelligence" are you talking about? I see nothing in HDMI 1.4a or earlier that requires doing stuff in kernelspace besides managing control to the hardware, but I might be missing something. In my opinion ARC is just a control mechanism, and can be dealt with in user- space, since you really want to just have hints about when ARC is enabled/disabled to take appropriate actions on the audio outputs or your system. > > I've started working on this API but I am still at the stage of playing > around with it and thinking about the best way this functionality should > be exposed. At least I managed to get the first CEC packets transferred > today :-) > > It will probably be a few weeks before I post something, but in the meantime > if you want to use CEC and have certain requirements that need to be met, > please let me know. If only so that I can be certain I haven't forgotten > anything. Here is my wish-list, if I may: - allow for a CEC adapter to be in "detached" / "attached" mode, particularly useful if the hardware doing CEC can process a basic set of messages to act a a global wake-up source for the system - allow for a CEC adapter to define several receive modes: unicast and "promiscuous", which is useful for dumping the CEC bus messages - make the CEC adapter API asynchronous for the data path, so it is easy for a driver to report completion of a successfully transmitted/received CEC frame Thank you! -- Florian
[PATCH 00/11] page-flip cleanups and fixes
On Mon, Oct 8, 2012 at 3:50 PM, Rob Clark wrote: > From: Rob Clark > > Add a helper fxn to send vblank event after pageflip, plus a handful > of fixes for other issues that I noticed in the process. Looks like a nice clean-up to me. Patches 1 and 4 are: Reviewed-by: Alex Deucher > > Other than OMAP, the changes are just compile tested, so would be > good to get a quick look from the maintainers of various drivers. > > Rob Clark (11): > drm: add drm_send_vblank_event() helper > drm/i915: use drm_send_vblank_event() helper > drm/nouveau: use drm_send_vblank_event() helper > drm/radeon: use drm_send_vblank_event() helper > drm/exynos: use drm_send_vblank_event() helper > drm/exynos: page flip fixes > drm/shmob: use drm_send_vblank_event() helper > drm/imx: use drm_send_vblank_event() helper > drm/imx: page flip fixes > drm/omap: use drm_send_vblank_event() helper > drm/omap: page-flip fixes > > drivers/gpu/drm/drm_irq.c | 65 > +++-- > drivers/gpu/drm/exynos/exynos_drm_crtc.c |1 - > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 10 + > drivers/gpu/drm/exynos/exynos_drm_vidi.c | 10 + > drivers/gpu/drm/exynos/exynos_mixer.c |9 +--- > drivers/gpu/drm/i915/intel_display.c | 15 +-- > drivers/gpu/drm/nouveau/nouveau_display.c | 13 +- > drivers/gpu/drm/radeon/radeon_display.c | 13 ++ > drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 19 ++--- > drivers/staging/imx-drm/ipuv3-crtc.c | 32 -- > drivers/staging/omapdrm/omap_crtc.c | 37 > include/drm/drmP.h|2 + > 12 files changed, 78 insertions(+), 148 deletions(-) > > -- > 1.7.9.5 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/radeon: allocate page tables on demand v3
On Son, 2012-10-07 at 15:06 +0200, Christian K?nig wrote: > Based on Dmitries work, but splitting the code into page > directory and page table handling makes it far more > readable and (hopefully) more reliable. > > Allocations of page tables are made from the SA on demand, > that should still work fine since all page tables are of > the same size. > > Also using the fact that allocations from the SA are mostly > continuously (except for end of buffer wraps and under very > high memory pressure) to group updates send to the chipset > specific code into larger chunks. > > v3: mostly a rewrite of Dmitries previous patch. > > Signed-off-by: Dmitry Cherkasov > Signed-off-by: Christian K?nig Tested-by: Michel D?nzer There are some checkpatch errors and warnings though, e.g. trailing whitespace. Also, I noticed a few typos: > + /* initial clear the page directory */ 'Initially' > @@ -865,6 +906,155 @@ uint64_t radeon_vm_map_gart(struct radeon_device *rdev, > uint64_t addr) > } > > /** > + * radeon_vm_update_pdes - make sure that page directory is valid > + * > + * @rdev: radeon_device pointer > + * @vm: requested vm > + * @start: start of GPU address range > + * @end: end of GPU address range > + * > + * Allocates new page tables if nessasary 'necessary' -- Earthling Michel D?nzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer
[PATCH v3 3/3] Android.mk: use LOCAL_COPY_HEADERS to export headers.
From: Haitao HuangExport necessary header files used by other components for Android, such as libva intel-driver, gralloc, hwcomposer, etc. Signed-off-by: Haitao Huang Signed-off-by: Chad Versace --- [chad: Fixed inconsistent indentation.] Android.mk | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Android.mk b/Android.mk index 1029dc6..68db5df 100644 --- a/Android.mk +++ b/Android.mk @@ -1,5 +1,5 @@ # -# Copyright ? 2011 Intel Corporation +# Copyright ? 2011-2012 Intel Corporation # # Permission is hereby granted, free of charge, to any person obtaining a # copy of this software and associated documentation files (the "Software"), @@ -45,6 +45,16 @@ LOCAL_C_INCLUDES := \ LOCAL_CFLAGS := \ -DHAVE_LIBDRM_ATOMIC_PRIMITIVES=1 +LOCAL_COPY_HEADERS :=\ + xf86drm.h\ + include/drm/drm_fourcc.h \ + include/drm/drm.h\ + include/drm/drm_mode.h \ + include/drm/drm_sarea.h \ + include/drm/i915_drm.h \ + intel/intel_bufmgr.h \ + +LOCAL_COPY_HEADERS_TO := libdrm include $(BUILD_SHARED_LIBRARY) include $(LOCAL_PATH)/intel/Android.mk -- 1.7.11.7
[PATCH] Android.mk: use LOCAL_COPY_HEADERS to export headers. v2
On 10/08/2012 02:43 PM, Oliver McFadden wrote: > On Mon, Oct 08, 2012 at 12:23:56PM +0300, Tapani P?lli wrote: >> From: Haitao Huang >> >> Export necessary header files used by other components for Android, >> such as libva intel-driver, gralloc, hwcomposer, etc. >> >> Signed-off-by: Haitao Huang >> [chad: Fixed inconsistent indentation.] >> Signed-off-by: Chad Versace >> --- > > Normal syntax is "Subject: [PATCH v2 N/M] ..." You should add a comment > here describing the v1..v2 diff. ok, I took away the gerrit 'change-id' row, I'll send v3 with your suggested changes. >> Android.mk | 12 +++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/Android.mk b/Android.mk >> index 1029dc6..68db5df 100644 >> --- a/Android.mk >> +++ b/Android.mk >> @@ -1,5 +1,5 @@ >> # >> -# Copyright ? 2011 Intel Corporation >> +# Copyright ? 2011-2012 Intel Corporation >> # >> # Permission is hereby granted, free of charge, to any person obtaining a >> # copy of this software and associated documentation files (the "Software"), >> @@ -45,6 +45,16 @@ LOCAL_C_INCLUDES := \ >> LOCAL_CFLAGS := \ >> -DHAVE_LIBDRM_ATOMIC_PRIMITIVES=1 >> >> +LOCAL_COPY_HEADERS :=\ >> +xf86drm.h\ >> +include/drm/drm_fourcc.h \ >> +include/drm/drm.h\ >> +include/drm/drm_mode.h \ >> +include/drm/drm_sarea.h \ >> +include/drm/i915_drm.h \ >> +intel/intel_bufmgr.h \ >> + >> +LOCAL_COPY_HEADERS_TO := libdrm >> include $(BUILD_SHARED_LIBRARY) >> >> include $(LOCAL_PATH)/intel/Android.mk >> -- >> 1.7.11.7 >> >> ___ >> dri-devel mailing list >> dri-devel at lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > -- // Tapani -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 551 bytes Desc: OpenPGP digital signature URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121008/8c2f2a8a/attachment.pgp>
[PATCH 1/2 v6] of: add helper to parse display timings
On Mon, 2012-10-08 at 14:04 +0200, Laurent Pinchart wrote: > Hi Tomi, > > On Monday 08 October 2012 12:01:18 Tomi Valkeinen wrote: > > On Mon, 2012-10-08 at 10:25 +0200, Guennadi Liakhovetski wrote: > > > In general, I might be misunderstanding something, but don't we have to > > > distinguish between 2 types of information about display timings: (1) is > > > defined by the display controller requirements, is known to the display > > > driver and doesn't need to be present in timings DT. We did have some of > > > these parameters in board data previously, because we didn't have proper > > > display controller drivers... (2) is board specific configuration, and is > > > such it has to be present in DT. > > > > > > In that way, doesn't "interlaced" belong to type (1) and thus doesn't need > > > to be present in DT? > > > > As I see it, this DT data is about the display (most commonly LCD > > panel), i.e. what video mode(s) the panel supports. If things were done > > my way, the panel's supported timings would be defined in the driver for > > the panel, and DT would be left to describe board specific data, but > > this approach has its benefits. > > What about dumb DPI panels ? They will all be supported by a single driver, > would you have the driver contain information about all known DPI panels ? DT > seems a good solution to me in this case. Yes, I would have a table in the driver for all the devices it supports, which would describe the device specific parameters. But I don't have a problem with DT solution. Both methods have their pros and cons, and perhaps DT based solution is more practical. > For complex panels where the driver will support a single (or very few) model > I agree that specifying the timings in DT isn't needed. > > > Thus, if you connect an interlaced panel to your board, you need to tell > > the display controller that this panel requires interlace signal. Also, > > pixel clock source doesn't make sense in this context, as this doesn't > > describe the actual used configuration, but only what the panel > > supports. > > > > Of course, if this is about describing the hardware, the default-mode > > property doesn't really fit in... > > Maybe we should rename it to native-mode then ? Hmm, right, if it means native mode, then it is describing the hardware. But would it make sense to require that the native mode is the first mode in the list, then? This would make the separate default-mode/native-mode property not needed. Tomi -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121008/f666c5ef/attachment.pgp>
[PATCH 11/11] drm/omap: page-flip fixes
From: Rob ClarkUserspace might not request a vblank event. So it is not an error for 'event' to be NULL, and we shouldn't use it to determine if there is a pending flip already. Signed-off-by: Rob Clark --- drivers/staging/omapdrm/omap_crtc.c |6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/staging/omapdrm/omap_crtc.c b/drivers/staging/omapdrm/omap_crtc.c index 74e019a..317b854 100644 --- a/drivers/staging/omapdrm/omap_crtc.c +++ b/drivers/staging/omapdrm/omap_crtc.c @@ -119,7 +119,6 @@ static void vblank_cb(void *arg) struct omap_crtc *omap_crtc = to_omap_crtc(crtc); unsigned long flags; - WARN_ON(!event); spin_lock_irqsave(>event_lock, flags); /* wakeup userspace */ @@ -127,6 +126,7 @@ static void vblank_cb(void *arg) drm_send_vblank_event(dev, -1, omap_crtc->event); omap_crtc->event = NULL; + omap_crtc->old_fb = NULL; spin_unlock_irqrestore(>event_lock, flags); } @@ -138,8 +138,6 @@ static void page_flip_cb(void *arg) struct drm_framebuffer *old_fb = omap_crtc->old_fb; struct drm_gem_object *bo; - omap_crtc->old_fb = NULL; - omap_crtc_mode_set_base(crtc, crtc->x, crtc->y, old_fb); /* really we'd like to setup the callback atomically w/ setting the @@ -162,7 +160,7 @@ static int omap_crtc_page_flip_locked(struct drm_crtc *crtc, DBG("%d -> %d", crtc->fb ? crtc->fb->base.id : -1, fb->base.id); - if (omap_crtc->event) { + if (omap_crtc->old_fb) { dev_err(dev->dev, "already a pending flip\n"); return -EINVAL; } -- 1.7.9.5
[PATCH 10/11] drm/omap: use drm_send_vblank_event() helper
From: Rob ClarkSigned-off-by: Rob Clark --- drivers/staging/omapdrm/omap_crtc.c | 31 ++- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/drivers/staging/omapdrm/omap_crtc.c b/drivers/staging/omapdrm/omap_crtc.c index 732f2ad..74e019a 100644 --- a/drivers/staging/omapdrm/omap_crtc.c +++ b/drivers/staging/omapdrm/omap_crtc.c @@ -114,40 +114,21 @@ static void omap_crtc_load_lut(struct drm_crtc *crtc) static void vblank_cb(void *arg) { - static uint32_t sequence = 0; struct drm_crtc *crtc = arg; struct drm_device *dev = crtc->dev; struct omap_crtc *omap_crtc = to_omap_crtc(crtc); - struct drm_pending_vblank_event *event = omap_crtc->event; unsigned long flags; - struct timeval now; WARN_ON(!event); + spin_lock_irqsave(>event_lock, flags); + + /* wakeup userspace */ + if (omap_crtc->event) + drm_send_vblank_event(dev, -1, omap_crtc->event); omap_crtc->event = NULL; - /* wakeup userspace */ - if (event) { - do_gettimeofday(); - - spin_lock_irqsave(>event_lock, flags); - /* TODO: we can't yet use the vblank time accounting, -* because omapdss lower layer is the one that knows -* the irq # and registers the handler, which more or -* less defeats how drm_irq works.. for now just fake -* the sequence number and use gettimeofday.. -* - event->event.sequence = drm_vblank_count_and_time( - dev, omap_crtc->id, ); -*/ - event->event.sequence = sequence++; - event->event.tv_sec = now.tv_sec; - event->event.tv_usec = now.tv_usec; - list_add_tail(>base.link, - >base.file_priv->event_list); - wake_up_interruptible(>base.file_priv->event_wait); - spin_unlock_irqrestore(>event_lock, flags); - } + spin_unlock_irqrestore(>event_lock, flags); } static void page_flip_cb(void *arg) -- 1.7.9.5
[PATCH 09/11] drm/imx: page flip fixes
From: Rob ClarkThe 'event' could be null, so don't list_del(>base.link).. and in fact even if it wasn't null there is no reason to do this. Also, this looks racy with the irq handler, so throw in a spinlock for good measure. Signed-off-by: Rob Clark --- drivers/staging/imx-drm/ipuv3-crtc.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c index 8fa0f4d..6745766 100644 --- a/drivers/staging/imx-drm/ipuv3-crtc.c +++ b/drivers/staging/imx-drm/ipuv3-crtc.c @@ -135,23 +135,26 @@ static int ipu_page_flip(struct drm_crtc *crtc, struct drm_pending_vblank_event *event) { struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc); + struct drm_device *drm = ipu_crtc->base.dev; + unsigned long flags; int ret; if (ipu_crtc->newfb) return -EBUSY; + spin_lock_irqsave(>event_lock, flags); ret = imx_drm_crtc_vblank_get(ipu_crtc->imx_crtc); if (ret) { dev_dbg(ipu_crtc->dev, "failed to acquire vblank counter\n"); - list_del(>base.link); - - return ret; + goto out; } ipu_crtc->newfb = fb; ipu_crtc->page_flip_event = event; - return 0; +out: + spin_unlock_irqrestore(>event_lock, flags); + return ret; } static const struct drm_crtc_funcs ipu_crtc_funcs = { -- 1.7.9.5
[PATCH 08/11] drm/imx: use drm_send_vblank_event() helper
From: Rob ClarkAlso, slightly changes the behavior to always put the vblank irq, even if userspace did not request a vblank event. As far as I can tell, the previous code would leak a vblank irq refcnt if userspace requested a pageflip without event. Signed-off-by: Rob Clark --- drivers/staging/imx-drm/ipuv3-crtc.c | 21 ++--- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c index 78d3eda..8fa0f4d 100644 --- a/drivers/staging/imx-drm/ipuv3-crtc.c +++ b/drivers/staging/imx-drm/ipuv3-crtc.c @@ -311,31 +311,14 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc, static void ipu_crtc_handle_pageflip(struct ipu_crtc *ipu_crtc) { - struct drm_pending_vblank_event *e; - struct timeval now; unsigned long flags; struct drm_device *drm = ipu_crtc->base.dev; spin_lock_irqsave(>event_lock, flags); - - e = ipu_crtc->page_flip_event; - if (!e) { - spin_unlock_irqrestore(>event_lock, flags); - return; - } - - do_gettimeofday(); - e->event.sequence = 0; - e->event.tv_sec = now.tv_sec; - e->event.tv_usec = now.tv_usec; + if (ipu_crtc->page_flip_event) + drm_send_vblank_event(drm, -1, ipu_crtc->page_flip_event); ipu_crtc->page_flip_event = NULL; - imx_drm_crtc_vblank_put(ipu_crtc->imx_crtc); - - list_add_tail(>base.link, >base.file_priv->event_list); - - wake_up_interruptible(>base.file_priv->event_wait); - spin_unlock_irqrestore(>event_lock, flags); } -- 1.7.9.5
[PATCH 07/11] drm/shmob: use drm_send_vblank_event() helper
From: Rob ClarkSigned-off-by: Rob Clark --- drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 19 --- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c index 0e7a930..3ecb702 100644 --- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c +++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c @@ -451,27 +451,16 @@ void shmob_drm_crtc_finish_page_flip(struct shmob_drm_crtc *scrtc) { struct drm_pending_vblank_event *event; struct drm_device *dev = scrtc->crtc.dev; - struct timeval vblanktime; unsigned long flags; spin_lock_irqsave(>event_lock, flags); event = scrtc->event; scrtc->event = NULL; + if (event) { + drm_send_vblank_event(dev, 0, event); + drm_vblank_put(dev, 0); + } spin_unlock_irqrestore(>event_lock, flags); - - if (event == NULL) - return; - - event->event.sequence = drm_vblank_count_and_time(dev, 0, ); - event->event.tv_sec = vblanktime.tv_sec; - event->event.tv_usec = vblanktime.tv_usec; - - spin_lock_irqsave(>event_lock, flags); - list_add_tail(>base.link, >base.file_priv->event_list); - wake_up_interruptible(>base.file_priv->event_wait); - spin_unlock_irqrestore(>event_lock, flags); - - drm_vblank_put(dev, 0); } static int shmob_drm_crtc_page_flip(struct drm_crtc *crtc, -- 1.7.9.5
[PATCH 06/11] drm/exynos: page flip fixes
From: Rob ClarkThe event wouldn't be on any list at this point, so nothing to delete it from. Signed-off-by: Rob Clark --- drivers/gpu/drm/exynos/exynos_drm_crtc.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index abb1e2f..d17419c 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c @@ -225,7 +225,6 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc, ret = drm_vblank_get(dev, exynos_crtc->pipe); if (ret) { DRM_DEBUG("failed to acquire vblank counter\n"); - list_del(>base.link); goto out; } -- 1.7.9.5
[PATCH 05/11] drm/exynos: use drm_send_vblank_event() helper
From: Rob ClarkSigned-off-by: Rob Clark --- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 10 ++ drivers/gpu/drm/exynos/exynos_drm_vidi.c | 10 ++ drivers/gpu/drm/exynos/exynos_mixer.c|9 ++--- 3 files changed, 6 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index b19cd93..fe8fb78 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -587,7 +587,6 @@ static void fimd_finish_pageflip(struct drm_device *drm_dev, int crtc) { struct exynos_drm_private *dev_priv = drm_dev->dev_private; struct drm_pending_vblank_event *e, *t; - struct timeval now; unsigned long flags; bool is_checked = false; @@ -601,13 +600,8 @@ static void fimd_finish_pageflip(struct drm_device *drm_dev, int crtc) is_checked = true; - do_gettimeofday(); - e->event.sequence = 0; - e->event.tv_sec = now.tv_sec; - e->event.tv_usec = now.tv_usec; - - list_move_tail(>base.link, >base.file_priv->event_list); - wake_up_interruptible(>base.file_priv->event_wait); + list_del(>base.link); + drm_send_vblank_event(drm_dev, -1, e); } if (is_checked) { diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c index e364165..4549efb 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c @@ -370,7 +370,6 @@ static void vidi_finish_pageflip(struct drm_device *drm_dev, int crtc) { struct exynos_drm_private *dev_priv = drm_dev->dev_private; struct drm_pending_vblank_event *e, *t; - struct timeval now; unsigned long flags; bool is_checked = false; @@ -384,13 +383,8 @@ static void vidi_finish_pageflip(struct drm_device *drm_dev, int crtc) is_checked = true; - do_gettimeofday(); - e->event.sequence = 0; - e->event.tv_sec = now.tv_sec; - e->event.tv_usec = now.tv_usec; - - list_move_tail(>base.link, >base.file_priv->event_list); - wake_up_interruptible(>base.file_priv->event_wait); + list_del(>base.link); + drm_send_vblank_event(drm_dev, -1, e); } if (is_checked) { diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 25b97d5..325aefd 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -828,7 +828,6 @@ static void mixer_finish_pageflip(struct drm_device *drm_dev, int crtc) { struct exynos_drm_private *dev_priv = drm_dev->dev_private; struct drm_pending_vblank_event *e, *t; - struct timeval now; unsigned long flags; bool is_checked = false; @@ -841,13 +840,9 @@ static void mixer_finish_pageflip(struct drm_device *drm_dev, int crtc) continue; is_checked = true; - do_gettimeofday(); - e->event.sequence = 0; - e->event.tv_sec = now.tv_sec; - e->event.tv_usec = now.tv_usec; - list_move_tail(>base.link, >base.file_priv->event_list); - wake_up_interruptible(>base.file_priv->event_wait); + list_del(>base.link); + drm_send_vblank_event(drm_dev, -1, e); } if (is_checked) -- 1.7.9.5
[PATCH 04/11] drm/radeon: use drm_send_vblank_event() helper
From: Rob ClarkSigned-off-by: Rob Clark --- drivers/gpu/drm/radeon/radeon_display.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index 7ddef8f..8dad9c2 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -271,8 +271,6 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id) { struct radeon_crtc *radeon_crtc = rdev->mode_info.crtcs[crtc_id]; struct radeon_unpin_work *work; - struct drm_pending_vblank_event *e; - struct timeval now; unsigned long flags; u32 update_pending; int vpos, hpos; @@ -328,14 +326,9 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id) radeon_crtc->unpin_work = NULL; /* wakeup userspace */ - if (work->event) { - e = work->event; - e->event.sequence = drm_vblank_count_and_time(rdev->ddev, crtc_id, ); - e->event.tv_sec = now.tv_sec; - e->event.tv_usec = now.tv_usec; - list_add_tail(>base.link, >base.file_priv->event_list); - wake_up_interruptible(>base.file_priv->event_wait); - } + if (work->event) + drm_send_vblank_event(rdev->ddev, crtc_id, work->event); + spin_unlock_irqrestore(>ddev->event_lock, flags); drm_vblank_put(rdev->ddev, radeon_crtc->crtc_id); -- 1.7.9.5
[PATCH 03/11] drm/nouveau: use drm_send_vblank_event() helper
From: Rob ClarkSigned-off-by: Rob Clark --- drivers/gpu/drm/nouveau/nouveau_display.c | 13 ++--- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 7e16dc5..b1a32b9 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -566,17 +566,8 @@ nouveau_finish_page_flip(struct nouveau_channel *chan, } s = list_first_entry(>flip, struct nouveau_page_flip_state, head); - if (s->event) { - struct drm_pending_vblank_event *e = s->event; - struct timeval now; - - do_gettimeofday(); - e->event.sequence = 0; - e->event.tv_sec = now.tv_sec; - e->event.tv_usec = now.tv_usec; - list_add_tail(>base.link, >base.file_priv->event_list); - wake_up_interruptible(>base.file_priv->event_wait); - } + if (s->event) + drm_send_vblank_event(dev, -1, s->event); list_del(>head); if (ps) -- 1.7.9.5
[PATCH 02/11] drm/i915: use drm_send_vblank_event() helper
From: Rob ClarkSigned-off-by: Rob Clark --- drivers/gpu/drm/i915/intel_display.c | 15 ++- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f5bcf6f..4716c83 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6158,8 +6158,6 @@ static void do_intel_finish_page_flip(struct drm_device *dev, struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_unpin_work *work; struct drm_i915_gem_object *obj; - struct drm_pending_vblank_event *e; - struct timeval tvbl; unsigned long flags; /* Ignore early vblank irqs */ @@ -6175,17 +6173,8 @@ static void do_intel_finish_page_flip(struct drm_device *dev, intel_crtc->unpin_work = NULL; - if (work->event) { - e = work->event; - e->event.sequence = drm_vblank_count_and_time(dev, intel_crtc->pipe, ); - - e->event.tv_sec = tvbl.tv_sec; - e->event.tv_usec = tvbl.tv_usec; - - list_add_tail(>base.link, - >base.file_priv->event_list); - wake_up_interruptible(>base.file_priv->event_wait); - } + if (work->event) + drm_send_vblank_event(dev, intel_crtc->pipe, work->event); drm_vblank_put(dev, intel_crtc->pipe); -- 1.7.9.5
[PATCH 01/11] drm: add drm_send_vblank_event() helper
From: Rob ClarkA helper that drivers can use to send vblank event after a pageflip. If the driver doesn't support proper vblank irq based time/seqn then just pass -1 for the pipe # to get do_gettimestamp() behavior (since there are a lot of drivers that don't use drm_vblank_count_and_time()) Also an internal send_vblank_event() helper for the various other code paths within drm_irq that also need to send vblank events. Signed-off-by: Rob Clark --- drivers/gpu/drm/drm_irq.c | 65 + include/drm/drmP.h|2 ++ 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 076c4a8..7a00d94 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -802,6 +802,43 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, int crtc, } EXPORT_SYMBOL(drm_vblank_count_and_time); +static void send_vblank_event(struct drm_pending_vblank_event *e, + unsigned long seq, struct timeval *now) +{ + e->event.sequence = seq; + e->event.tv_sec = now->tv_sec; + e->event.tv_usec = now->tv_usec; + + list_add_tail(>base.link, + >base.file_priv->event_list); + wake_up_interruptible(>base.file_priv->event_wait); + trace_drm_vblank_event_delivered(e->base.pid, e->pipe, +e->event.sequence); +} + +/** + * drm_send_vblank_event - helper to send vblank event after pageflip + * @dev: DRM device + * @crtc: CRTC in question + * @e: the event to send + * + * Updates sequence # and timestamp on event, and sends it to userspace. + */ +void drm_send_vblank_event(struct drm_device *dev, int crtc, + struct drm_pending_vblank_event *e) +{ + struct timeval now; + unsigned int seq; + if (crtc >= 0) { + seq = drm_vblank_count_and_time(dev, crtc, ); + } else { + seq = 0; + do_gettimeofday(); + } + send_vblank_event(e, seq, ); +} +EXPORT_SYMBOL(drm_send_vblank_event); + /** * drm_update_vblank_count - update the master vblank counter * @dev: DRM device @@ -955,15 +992,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc) DRM_DEBUG("Sending premature vblank event on disable: \ wanted %d, current %d\n", e->event.sequence, seq); - - e->event.sequence = seq; - e->event.tv_sec = now.tv_sec; - e->event.tv_usec = now.tv_usec; + list_del(>base.link); drm_vblank_put(dev, e->pipe); - list_move_tail(>base.link, >base.file_priv->event_list); - wake_up_interruptible(>base.file_priv->event_wait); - trace_drm_vblank_event_delivered(e->base.pid, e->pipe, -e->event.sequence); + send_vblank_event(e, seq, ); } spin_unlock_irqrestore(>vbl_lock, irqflags); @@ -1107,15 +1138,8 @@ static int drm_queue_vblank_event(struct drm_device *dev, int pipe, e->event.sequence = vblwait->request.sequence; if ((seq - vblwait->request.sequence) <= (1 << 23)) { - e->event.sequence = seq; - e->event.tv_sec = now.tv_sec; - e->event.tv_usec = now.tv_usec; drm_vblank_put(dev, pipe); - list_add_tail(>base.link, >base.file_priv->event_list); - wake_up_interruptible(>base.file_priv->event_wait); - vblwait->reply.sequence = seq; - trace_drm_vblank_event_delivered(current->pid, pipe, -vblwait->request.sequence); + send_vblank_event(e, seq, ); } else { /* drm_handle_vblank_events will call drm_vblank_put */ list_add_tail(>base.link, >vblank_event_list); @@ -1256,14 +1280,9 @@ static void drm_handle_vblank_events(struct drm_device *dev, int crtc) DRM_DEBUG("vblank event on %d, current %d\n", e->event.sequence, seq); - e->event.sequence = seq; - e->event.tv_sec = now.tv_sec; - e->event.tv_usec = now.tv_usec; + list_del(>base.link); drm_vblank_put(dev, e->pipe); - list_move_tail(>base.link, >base.file_priv->event_list); - wake_up_interruptible(>base.file_priv->event_wait); - trace_drm_vblank_event_delivered(e->base.pid, e->pipe, -e->event.sequence); + send_vblank_event(e, seq, ); } spin_unlock_irqrestore(>event_lock, flags); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 05af0e7..ee8f927 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1437,6 +1437,8 @@ extern int drm_vblank_wait(struct drm_device
[PATCH 00/11] page-flip cleanups and fixes
From: Rob ClarkAdd a helper fxn to send vblank event after pageflip, plus a handful of fixes for other issues that I noticed in the process. Other than OMAP, the changes are just compile tested, so would be good to get a quick look from the maintainers of various drivers. Rob Clark (11): drm: add drm_send_vblank_event() helper drm/i915: use drm_send_vblank_event() helper drm/nouveau: use drm_send_vblank_event() helper drm/radeon: use drm_send_vblank_event() helper drm/exynos: use drm_send_vblank_event() helper drm/exynos: page flip fixes drm/shmob: use drm_send_vblank_event() helper drm/imx: use drm_send_vblank_event() helper drm/imx: page flip fixes drm/omap: use drm_send_vblank_event() helper drm/omap: page-flip fixes drivers/gpu/drm/drm_irq.c | 65 +++-- drivers/gpu/drm/exynos/exynos_drm_crtc.c |1 - drivers/gpu/drm/exynos/exynos_drm_fimd.c | 10 + drivers/gpu/drm/exynos/exynos_drm_vidi.c | 10 + drivers/gpu/drm/exynos/exynos_mixer.c |9 +--- drivers/gpu/drm/i915/intel_display.c | 15 +-- drivers/gpu/drm/nouveau/nouveau_display.c | 13 +- drivers/gpu/drm/radeon/radeon_display.c | 13 ++ drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 19 ++--- drivers/staging/imx-drm/ipuv3-crtc.c | 32 -- drivers/staging/omapdrm/omap_crtc.c | 37 include/drm/drmP.h|2 + 12 files changed, 78 insertions(+), 148 deletions(-) -- 1.7.9.5
[PATCH 2/2 v6] of: add generic videomode description
On Mon, Oct 08, 2012 at 02:13:50PM +0200, Laurent Pinchart wrote: > Hi Steffen, > > Thanks for the patch. > > On Thursday 04 October 2012 19:59:20 Steffen Trumtrar wrote: > > Get videomode from devicetree in a format appropriate for the > > backend. drm_display_mode and fb_videomode are supported atm. > > Uses the display signal timings from of_display_timings > > > > Signed-off-by: Steffen Trumtrar > > --- > > drivers/of/Kconfig |5 + > > drivers/of/Makefile |1 + > > drivers/of/of_videomode.c| 212 +++ > > include/linux/of_videomode.h | 41 > > 4 files changed, 259 insertions(+) > > create mode 100644 drivers/of/of_videomode.c > > create mode 100644 include/linux/of_videomode.h > > > > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig > > index 646deb0..74282e2 100644 > > --- a/drivers/of/Kconfig > > +++ b/drivers/of/Kconfig > > @@ -88,4 +88,9 @@ config OF_DISPLAY_TIMINGS > > help > > helper to parse display timings from the devicetree > > > > +config OF_VIDEOMODE > > + def_bool y > > + help > > + helper to get videomodes from the devicetree > > + > > endmenu # OF > > diff --git a/drivers/of/Makefile b/drivers/of/Makefile > > index c8e9603..09d556f 100644 > > --- a/drivers/of/Makefile > > +++ b/drivers/of/Makefile > > @@ -12,3 +12,4 @@ obj-$(CONFIG_OF_PCI) += of_pci.o > > obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o > > obj-$(CONFIG_OF_MTD) += of_mtd.o > > obj-$(CONFIG_OF_DISPLAY_TIMINGS) += of_display_timings.o > > +obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o > > diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c > > new file mode 100644 > > index 000..76ac16e > > --- /dev/null > > +++ b/drivers/of/of_videomode.c > > @@ -0,0 +1,212 @@ > > +/* > > + * generic videomode helper > > + * > > + * Copyright (c) 2012 Steffen Trumtrar , > > Pengutronix > > + * > > + * This file is released under the GPLv2 > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +void dump_fb_videomode(struct fb_videomode *m) > > +{ > > +pr_debug("fb_videomode = %d %d %d %d %d %d %d %d %d %d %d %d %d\n", > > That's going to be pretty difficult to read :-) Would it make sense to group > several attributes logically (for instance using %ux%u for m->xres, m->yres) ? > No problem. That can be changed. > > +m->refresh, m->xres, m->yres, m->pixclock, m->left_margin, > > +m->right_margin, m->upper_margin, m->lower_margin, + > > m->hsync_len, m->vsync_len, m->sync, m->vmode, m->flag); > > +} > > Shouldn't this (and the other non exported functions below) be static ? > Yes. > > +void dump_drm_displaymode(struct drm_display_mode *m) > > +{ > > +pr_debug("drm_displaymode = %d %d %d %d %d %d %d %d %d\n", > > +m->hdisplay, m->hsync_start, m->hsync_end, m->htotal, > > +m->vdisplay, m->vsync_start, m->vsync_end, m->vtotal, > > +m->clock); > > +} > > + > > +int videomode_from_timing(struct display_timings *disp, struct videomode > > *vm, > > + int index) > > +{ > > + struct signal_timing *st = NULL; > > + > > + if (!vm) > > + return -EINVAL; > > + > > What about making vm a mandatory argument ? It looks to me like a caller bug > if vm is NULL. > The caller must provide the struct videomode, yes. Wouldn't the kernel hang itself with a NULL pointer exception, if I just work with it ? > > + st = display_timings_get(disp, index); > > + > > You can remove the blank line. > > > + if (!st) { > > + pr_err("%s: no signal timings found\n", __func__); > > + return -EINVAL; > > + } > > + > > + vm->pixelclock = signal_timing_get_value(>pixelclock, 0); > > + vm->hactive = signal_timing_get_value(>hactive, 0); > > + vm->hfront_porch = signal_timing_get_value(>hfront_porch, 0); > > + vm->hback_porch = signal_timing_get_value(>hback_porch, 0); > > + vm->hsync_len = signal_timing_get_value(>hsync_len, 0); > > + > > + vm->vactive = signal_timing_get_value(>vactive, 0); > > + vm->vfront_porch = signal_timing_get_value(>vfront_porch, 0); > > + vm->vback_porch = signal_timing_get_value(>vback_porch, 0); > > + vm->vsync_len = signal_timing_get_value(>vsync_len, 0); > > + > > + vm->vah = st->vsync_pol_active_high; > > + vm->hah = st->hsync_pol_active_high; > > + vm->interlaced = st->interlaced; > > + vm->doublescan = st->doublescan; > > + > > + return 0; > > +} > > + > > +int of_get_videomode(struct device_node *np, struct videomode *vm, int > > index) > > I wonder how to avoid abuse of this functions. It's a useful helper for > drivers that need to get a video mode once only, but would result in lower > performances if a driver calls it for every mode. Drivers must call > of_get_display_timing_list instead in that case and case the display timings. > I'm
[RFC 0/4] drm: add raw monotonic timestamp support (Imre Deak)
On Sat, 2012-10-06 at 04:46 +0200, Mario Kleiner wrote: > On 05.10.12 15:37, intel-gfx-request at lists.freedesktop.org wrote: > > > > Today's Topics: > > > > 1. [RFC 0/4] drm: add raw monotonic timestamp support (Imre Deak) > > 2. [RFC 1/4] time: export getnstime_raw_and_real for DRM (Imre Deak) > > 3. [RFC 2/4] drm: make memset/calloc for _vblank_time more > >robust (Imre Deak) > > 4. [RFC 3/4] drm: use raw time in > >drm_calc_vbltimestamp_from_scanoutpos (Imre Deak) > > 5. [RFC 4/4] drm: add support for raw monotonic vblank > >timestamps (Imre Deak) > > > > > > -- > > > > Message: 1 > > Date: Fri, 5 Oct 2012 16:36:58 +0300 > > From: Imre Deak > > To: Daniel Vetter, Chris Wilson > > , Kristian H?gsberg > > Cc:intel-gfx at lists.freedesktop.org,dri-devel at lists.freedesktop.org > > Subject: [Intel-gfx] [RFC 0/4] drm: add raw monotonic timestamp > > support > > Message-ID:<1349444222-22274-1-git-send-email-imre.deak at intel.com> > > > > This is needed to make applications depending on vblank/page flip > > timestamps independent of time ajdustments. > > > > I've tested these with an updated intel-gpu-test/flip_test and will send > > the update for that once there's no objection about this patchset. > > > > I'm mostly fine with this, although the wall time compatibility stuff > may not be useful given that userspace apps, e.g., OpenGL clients, have > no way to actually ask for wall vs. monotonic, and the spec actually > expects them to expect monotonic timestamps. > > I also see that an update to nouveau-kms is missing? Afaik the vblank > timestamping on nouveau-kms is still handled by the fallback in the drm, > but pageflip completion uses do_gettimeofday() for the timestamping and > returns a hard-coded zero vblank count all time for pageflip events > (yay!). Lucas Stach and me have written and tested some patches to fix > this over a year ago, but somehow they never made it into the kms > driver, mostly due to bad timing in when stuff was submitted, reviewed > and revised, not for serious technical objections iirc. > > Ideally we could give them another try, or at least update nouveaus > pageflip timestamping to avoid really bad breakage for OpenGL clients or > the nouveau-ddx due to inconsistent vblank vs. pageflip timestamps. > > I quickly looked over the patches, i think they look mostly good, see > some small comments below. > > > Subject: [Intel-gfx] [RFC 3/4] drm: use raw time in > > drm_calc_vbltimestamp_from_scanoutpos > > Message-ID: <1349444222-22274-4-git-send-email-imre.deak at intel.com> > > > > The timestamp is used here for handling the timeout case, so we don't > > want it to be affected by time adjustments. > > > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/drm_irq.c | 13 +++-- > > 1 file changed, 7 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > > index 77f6577..5e42981 100644 > > --- a/drivers/gpu/drm/drm_irq.c > > +++ b/drivers/gpu/drm/drm_irq.c > > @@ -576,7 +576,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct > > drm_device *dev, int crtc, > > unsigned flags, > > struct drm_crtc *refcrtc) > > { > > - struct timeval stime, raw_time; > > + struct timespec raw_stime, raw_etime, real_etime; > > struct drm_display_mode *mode; > > int vbl_status, vtotal, vdisplay; > > int vpos, hpos, i; > > @@ -625,13 +625,13 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct > > drm_device *dev, int crtc, > > preempt_disable(); > > > > /* Get system timestamp before query. */ > > - do_gettimeofday(); > > + getrawmonotonic(_stime); > > > > /* Get vertical and horizontal scanout pos. vpos, hpos. */ > > vbl_status = dev->driver->get_scanout_position(dev, crtc, > > , ); > > > > /* Get system timestamp after query. */ > > - do_gettimeofday(_time); > > + getnstime_raw_and_real(_etime, _etime); > > > > preempt_enable(); > > > > @@ -642,7 +642,8 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct > > drm_device *dev, int crtc, > > return -EIO; > > } > > > > - duration_ns = timeval_to_ns(_time) - timeval_to_ns(); > > + duration_ns = timespec_to_ns(_etime) - > > + timespec_to_ns(_stime); > > > > /* Accept result with < max_error nsecs timing uncertainty. */ > > if (duration_ns <= (s64) *max_error) > > @@ -692,11 +693,11 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct > > drm_device *dev, int crtc, > > /* Subtract time delta from raw timestamp to get final > > * vblank_time timestamp for end of vblank. > > */ > > - *vblank_time =
[PATCH] Android.mk: use LOCAL_COPY_HEADERS to export headers. v2
On Mon, Oct 08, 2012 at 12:23:56PM +0300, Tapani P?lli wrote: > From: Haitao Huang > > Export necessary header files used by other components for Android, > such as libva intel-driver, gralloc, hwcomposer, etc. > > Signed-off-by: Haitao Huang > [chad: Fixed inconsistent indentation.] > Signed-off-by: Chad Versace > --- Normal syntax is "Subject: [PATCH v2 N/M] ..." You should add a comment here describing the v1..v2 diff. > Android.mk | 12 +++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/Android.mk b/Android.mk > index 1029dc6..68db5df 100644 > --- a/Android.mk > +++ b/Android.mk > @@ -1,5 +1,5 @@ > # > -# Copyright ? 2011 Intel Corporation > +# Copyright ? 2011-2012 Intel Corporation > # > # Permission is hereby granted, free of charge, to any person obtaining a > # copy of this software and associated documentation files (the "Software"), > @@ -45,6 +45,16 @@ LOCAL_C_INCLUDES := \ > LOCAL_CFLAGS := \ > -DHAVE_LIBDRM_ATOMIC_PRIMITIVES=1 > > +LOCAL_COPY_HEADERS :=\ > + xf86drm.h\ > + include/drm/drm_fourcc.h \ > + include/drm/drm.h\ > + include/drm/drm_mode.h \ > + include/drm/drm_sarea.h \ > + include/drm/i915_drm.h \ > + intel/intel_bufmgr.h \ > + > +LOCAL_COPY_HEADERS_TO := libdrm > include $(BUILD_SHARED_LIBRARY) > > include $(LOCAL_PATH)/intel/Android.mk > -- > 1.7.11.7 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Oliver McFadden.
[PATCH 2/3] libdrm,intel: Add Android makefiles (v2)
On Mon, Oct 08, 2012 at 08:50:25AM +0300, Tapani P?lli wrote: > From: Chad Versace > > This enables libdrm.so and libdrm_intel.so to build on Android > IceCreamSandwich. > > v2: Link libdrm_intel to libpciaccess. > > Signed-off-by: Chad Versace Reviewed-by: Oliver McFadden > --- > Android.mk | 52 +++ > intel/Android.mk | 57 > > 2 files changed, 109 insertions(+) > create mode 100644 Android.mk > create mode 100644 intel/Android.mk > > diff --git a/Android.mk b/Android.mk > new file mode 100644 > index 000..1029dc6 > --- /dev/null > +++ b/Android.mk > @@ -0,0 +1,52 @@ > +# > +# Copyright ? 2011 Intel Corporation > +# > +# Permission is hereby granted, free of charge, to any person obtaining a > +# copy of this software and associated documentation files (the "Software"), > +# to deal in the Software without restriction, including without limitation > +# the rights to use, copy, modify, merge, publish, distribute, sublicense, > +# and/or sell copies of the Software, and to permit persons to whom the > +# Software is furnished to do so, subject to the following conditions: > +# > +# The above copyright notice and this permission notice (including the next > +# paragraph) shall be included in all copies or substantial portions of the > +# Software. > +# > +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > DEALINGS > +# IN THE SOFTWARE. > +# > + > +LIBDRM_VALID_GPU_DRIVERS := i915 i965 > + > +# Skip this makefile if we're not building for a known GPU. > +ifneq ($(filter $(BOARD_GPU_DRIVERS), $(LIBDRM_VALID_GPU_DRIVERS)),) > + > +LOCAL_PATH := $(call my-dir) > +include $(CLEAR_VARS) > + > +LIBDRM_TOP := $(LOCAL_PATH) > + > +# Import variable LIBDRM_SOURCES. > +include $(LOCAL_PATH)/sources.mk > + > +LOCAL_MODULE := libdrm > +LOCAL_MODULE_TAGS := optional > + > +LOCAL_SRC_FILES := $(LIBDRM_SOURCES) > + > +LOCAL_C_INCLUDES := \ > + $(LIBDRM_TOP)/include/drm > + > +LOCAL_CFLAGS := \ > + -DHAVE_LIBDRM_ATOMIC_PRIMITIVES=1 > + > +include $(BUILD_SHARED_LIBRARY) > + > +include $(LOCAL_PATH)/intel/Android.mk > + > +endif # BOARD_GPU_DRIVERS > diff --git a/intel/Android.mk b/intel/Android.mk > new file mode 100644 > index 000..3647a14 > --- /dev/null > +++ b/intel/Android.mk > @@ -0,0 +1,57 @@ > +# > +# Copyright ? 2011 Intel Corporation > +# > +# Permission is hereby granted, free of charge, to any person obtaining a > +# copy of this software and associated documentation files (the "Software"), > +# to deal in the Software without restriction, including without limitation > +# the rights to use, copy, modify, merge, publish, distribute, sublicense, > +# and/or sell copies of the Software, and to permit persons to whom the > +# Software is furnished to do so, subject to the following conditions: > +# > +# The above copyright notice and this permission notice (including the next > +# paragraph) shall be included in all copies or substantial portions of the > +# Software. > +# > +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > DEALINGS > +# IN THE SOFTWARE. > +# > + > +LIBDRM_INTEL_VALID_GPU_DRIVERS := i915 i965 > + > +# Skip this makefile if we're not building for an Intel GPU. > +ifneq ($(filter $(BOARD_GPU_DRIVERS), $(LIBDRM_INTEL_VALID_GPU_DRIVERS)),) > + > +LOCAL_PATH := $(call my-dir) > +include $(CLEAR_VARS) > + > +# Import variable LIBDRM_INTEL_SOURCES. > +include $(LOCAL_PATH)/sources.mk > + > +LOCAL_MODULE := libdrm_intel > +LOCAL_MODULE_TAGS := optional > + > +LOCAL_SHARED_LIBRARIES := libdrm > + > +LOCAL_SRC_FILES := $(LIBDRM_INTEL_SOURCES) > + > +LOCAL_C_INCLUDES := \ > + $(LIBDRM_TOP) \ > + $(LIBDRM_TOP)/intel \ > + $(LIBDRM_TOP)/include/drm \ > + external/libpciaccess/include > + > +LOCAL_CFLAGS := \ > + -DHAVE_LIBDRM_ATOMIC_PRIMITIVES=1 > + > +LOCAL_SHARED_LIBRARIES := \ > + libdrm \ > + libpciaccess > + > +include $(BUILD_SHARED_LIBRARY) > + > +endif # BOARD_GPU_DRIVERS > -- > 1.7.11.7 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org >
[PATCH 1/3] libdrm,intel: Factor source file lists into sources.mk
On Mon, Oct 08, 2012 at 08:50:24AM +0300, Tapani P?lli wrote: > From: Chad Versace > > Factor the source file list for libdrm.so from Makefile.am into > sources.mk. Ditto for libdrm_intel.so. > > This is in preparation for adding support for Android. The sources.mk's > will be shared between autotools and Android. > > Rationale: The most commonly changed parts of any makefile are the source > lists. So, by sharing the lists between the two build systems, we can > reduce the frequency at which modifications to the Linux build breaks the > Android build. > > Signed-off-by: Chad Versace > Signed-off-by: Sean V Kelley > Signed-off-by: Tapani P?lli Reviewed-by: Oliver McFadden > --- > Makefile.am | 9 - > intel/Makefile.am | 9 - > intel/sources.mk | 30 ++ > sources.mk| 30 ++ > 4 files changed, 68 insertions(+), 10 deletions(-) > create mode 100644 intel/sources.mk > create mode 100644 sources.mk > > diff --git a/Makefile.am b/Makefile.am > index 8ecd9d9..b854703 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -49,6 +49,9 @@ if HAVE_EXYNOS > EXYNOS_SUBDIR = exynos > endif > > +# Import variable LIBDRM_SOURCES. > +include sources.mk > + > SUBDIRS = . $(LIBKMS_SUBDIR) $(INTEL_SUBDIR) $(NOUVEAU_SUBDIR) > $(RADEON_SUBDIR) $(OMAP_SUBDIR) $(EXYNOS_SUBDIR) tests include man > > libdrm_la_LTLIBRARIES = libdrm.la > @@ -59,11 +62,7 @@ libdrm_la_LIBADD = @CLOCK_LIB@ > libdrm_la_CPPFLAGS = -I$(top_srcdir)/include/drm > > libdrm_la_SOURCES = \ > - xf86drm.c \ > - xf86drmHash.c \ > - xf86drmRandom.c \ > - xf86drmSL.c \ > - xf86drmMode.c \ > + $(LIBDRM_SOURCES) \ > xf86atomic.h\ > libdrm_lists.h > > diff --git a/intel/Makefile.am b/intel/Makefile.am > index f49b099..e937c4b 100644 > --- a/intel/Makefile.am > +++ b/intel/Makefile.am > @@ -22,6 +22,9 @@ > # Authors: > #Eric Anholt > > +# Import variable LIBDRM_INTEL_SOURCES. > +include sources.mk > + > AM_CFLAGS = \ > $(WARN_CFLAGS) \ > -I$(top_srcdir) \ > @@ -40,13 +43,9 @@ libdrm_intel_la_LIBADD = ../libdrm.la \ > @CLOCK_LIB@ > > libdrm_intel_la_SOURCES = \ > - intel_bufmgr.c \ > + $(LIBDRM_INTEL_SOURCES) \ > intel_bufmgr_priv.h \ > - intel_bufmgr_fake.c \ > - intel_bufmgr_gem.c \ > - intel_decode.c \ > intel_chipset.h \ > - mm.c \ > mm.h > > intel_bufmgr_gem_o_CFLAGS = $(AM_CFLAGS) -c99 > diff --git a/intel/sources.mk b/intel/sources.mk > new file mode 100644 > index 000..2f6f744 > --- /dev/null > +++ b/intel/sources.mk > @@ -0,0 +1,30 @@ > +# > +# Copyright ? 2012 Intel Corporation > +# > +# Permission is hereby granted, free of charge, to any person obtaining a > +# copy of this software and associated documentation files (the "Software"), > +# to deal in the Software without restriction, including without limitation > +# the rights to use, copy, modify, merge, publish, distribute, sublicense, > +# and/or sell copies of the Software, and to permit persons to whom the > +# Software is furnished to do so, subject to the following conditions: > +# > +# The above copyright notice and this permission notice (including the next > +# paragraph) shall be included in all copies or substantial portions of the > +# Software. > +# > +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > DEALINGS > +# IN THE SOFTWARE. > +# > + > +# for libdrm_intel.so > +LIBDRM_INTEL_SOURCES := \ > + intel_bufmgr.c \ > + intel_bufmgr_fake.c \ > + intel_bufmgr_gem.c \ > + intel_decode.c \ > + mm.c > diff --git a/sources.mk b/sources.mk > new file mode 100644 > index 000..19aa059 > --- /dev/null > +++ b/sources.mk > @@ -0,0 +1,30 @@ > +# > +# Copyright ? 2012 Intel Corporation > +# > +# Permission is hereby granted, free of charge, to any person obtaining a > +# copy of this software and associated documentation files (the "Software"), > +# to deal in the Software without restriction, including without limitation > +# the rights to use, copy, modify, merge, publish, distribute, sublicense, > +# and/or sell copies of the Software, and to permit persons to whom the > +# Software is furnished to do so, subject to the following conditions: > +# > +# The above copyright notice and this permission notice (including the next > +# paragraph)
[PATCH 3/3] Android.mk: use LOCAL_COPY_HEADERS to export headers.
On Mon, Oct 08, 2012 at 08:50:26AM +0300, Tapani P?lli wrote: > From: Haitao Huang > > Export necessary header files used by other components for Android, > such as libva intel-driver, gralloc, hwcomposer, etc. > > Change-Id: I2feabf6941379ef4d756e942f30eba059de641f1 > Signed-off-by: Haitao Huang > [chad: Fixed inconsistent indentation.] > Signed-off-by: Chad Versace > --- Please put "[chad: Fixed inconsistent indentation.]" after the triple-dash, so that it will not be immortalized into the Git history. Otherwise, looks fine, does what it says on the box. Reviewed-by: Oliver McFadden > Android.mk | 12 +++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/Android.mk b/Android.mk > index 1029dc6..68db5df 100644 > --- a/Android.mk > +++ b/Android.mk > @@ -1,5 +1,5 @@ > # > -# Copyright ? 2011 Intel Corporation > +# Copyright ? 2011-2012 Intel Corporation > # > # Permission is hereby granted, free of charge, to any person obtaining a > # copy of this software and associated documentation files (the "Software"), > @@ -45,6 +45,16 @@ LOCAL_C_INCLUDES := \ > LOCAL_CFLAGS := \ > -DHAVE_LIBDRM_ATOMIC_PRIMITIVES=1 > > +LOCAL_COPY_HEADERS :=\ > + xf86drm.h\ > + include/drm/drm_fourcc.h \ > + include/drm/drm.h\ > + include/drm/drm_mode.h \ > + include/drm/drm_sarea.h \ > + include/drm/i915_drm.h \ > + intel/intel_bufmgr.h \ > + > +LOCAL_COPY_HEADERS_TO := libdrm > include $(BUILD_SHARED_LIBRARY) > > include $(LOCAL_PATH)/intel/Android.mk > -- > 1.7.11.7 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Oliver McFadden.
[RFC 1/4] time: export getnstime_raw_and_real for DRM
On Sat, 2012-10-06 at 03:41 +0200, Mario Kleiner wrote: > [...] > > But then Psychtoolbox checks each timestamp it gets from somewhere > "outside" (OML_sync_control / INTEL_swap_events / ALSA audio timestamps, > network receive timestamps, evdev, x11, ...) if it is in gettimeofday() > aka CLOCK_REALTIME aka wall time or in CLOCK_MONOTONIC time and just > remaps to whatever its reference clock is. > > There's no way around this than to have no fixed expectations, but to > remap stuff on the fly, because different parts of the Linux universe > have decided on different time bases, or even switched somewhere from > one kernel version to the next in the last years, e.g., ALSA, which at > some time switched from wall clock to CLOCK_MONOTONIC. Sometimes > clock_gettime() wasn't available at all in older setups, so there only > was gettimeofday(). Or toolkits like GStreamer use different timebases > dependent on OS and sometimes even on plugins. > > I would expect that other timing sensitive apps have to have ways to > handle this in similar ways. I think the question is, can we be sure? I don't think there is any guarantee that random application X will not be confused by an unconditional switch to monotonic timestamps. > Wrt. to the drm vblank/pageflip timestamps, the userspace extensions > which expose these (INTEL_swap_events, OML_sync_control) don't allow > apps to select which timebase to use, they define monotonic time as what > is returned, so i don't know how a userspace app could actually ask the > DRM for one or the other format? So i guess just switching to > CLOCK_MONOTONIC shouldn't be that bad. An application could just use the kernel DRM interface directly. I admit this is not very likely but this is what should determine the rules by which we change the ABI. > Kristian, i assume Wayland will also return presentation timestamps in > the format and microsecond precision of the DRM, right? > > On 05.10.12 18:22, intel-gfx-request at lists.freedesktop.org wrote: > > Message: 7 Date: Fri, 5 Oct 2012 12:14:29 -0400 From: Kristian H?gsberg > > ... > > I just had a quick look at driver/input/evdev.c, since evdev devices > > did a similar change recently to allow evdev timestamp from the > > monotonic clock. They're using a different time API: > > > > time_mono = ktime_get(); > > time_real = ktime_sub(time_mono, ktime_get_monotonic_offset()); > > > > and > > > > event->time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ? > > mono : real); > > > > I'm not really up-to-date on kernel time APIs, but I wonder if that > > may be better? At least, I suspect we wouldn't need changes outside > > drm if we use this API. > > > > Kristian > > Userspace apps only have access to what gettimeofday() and > clock_gettime() for CLOCK_REALTIME (== gettimeofday() afaik) and > CLOCK_MONOTONIC return, so whatever is returned should be in > CLOCK_MONOTONIC format, otherwise there will be lots of tears and dead > kittens. I think what evdev does makes a lot of sense, but i'm also not > up-to-date about the various layers of timing apis. Yes, this should be the case, regardless of which kernel interface we decide to use. --Imre
[PATCH] drm: move a few frequent traces to DRM_VERB()
From: Rob ClarkAdd a new bit for the trace-mask for verbose traces. Signed-off-by: Rob Clark --- drivers/gpu/drm/drm_drv.c |9 +++-- drivers/gpu/drm/drm_vm.c |4 ++-- include/drm/drmP.h|9 - 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 933e7ae..b31296f 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -394,7 +394,7 @@ long drm_ioctl(struct file *filp, atomic_inc(>counts[_DRM_STAT_IOCTLS]); ++file_priv->ioctl_count; - DRM_DEBUG("pid=%d, cmd=0x%02x, nr=0x%02x, dev 0x%lx, auth=%d\n", + DRM_VERB("pid=%d, cmd=0x%02x, nr=0x%02x, dev 0x%lx, auth=%d\n", task_pid_nr(current), cmd, nr, (long)old_encode_dev(file_priv->minor->device), file_priv->authenticated); @@ -475,8 +475,13 @@ long drm_ioctl(struct file *filp, if (kdata != stack_kdata) kfree(kdata); atomic_dec(>ioctl_count); - if (retcode) + if (retcode) { + DRM_DEBUG("pid=%d, cmd=0x%02x, nr=0x%02x, dev 0x%lx, auth=%d\n", + task_pid_nr(current), cmd, nr, + (long)old_encode_dev(file_priv->minor->device), + file_priv->authenticated); DRM_DEBUG("ret = %d\n", retcode); + } return retcode; } diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c index 961ee08..0a6ff73 100644 --- a/drivers/gpu/drm/drm_vm.c +++ b/drivers/gpu/drm/drm_vm.c @@ -411,7 +411,7 @@ void drm_vm_open_locked(struct drm_device *dev, { struct drm_vma_entry *vma_entry; - DRM_DEBUG("0x%08lx,0x%08lx\n", + DRM_VERB("0x%08lx,0x%08lx\n", vma->vm_start, vma->vm_end - vma->vm_start); atomic_inc(>vma_count); @@ -438,7 +438,7 @@ void drm_vm_close_locked(struct drm_device *dev, { struct drm_vma_entry *pt, *temp; - DRM_DEBUG("0x%08lx,0x%08lx\n", + DRM_VERB("0x%08lx,0x%08lx\n", vma->vm_start, vma->vm_end - vma->vm_start); atomic_dec(>vma_count); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index f719c4d..b472114 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -92,6 +92,8 @@ struct drm_device; #define DRM_UT_DRIVER 0x02 #define DRM_UT_KMS 0x04 #define DRM_UT_PRIME 0x08 +#define DRM_UT_CORE_VERB 0x10 + /* * Three debug levels are defined. * drm_core, drm_driver, drm_kms @@ -206,7 +208,11 @@ int drm_err(const char *func, const char *format, ...); drm_ut_debug_printk(DRM_UT_CORE, DRM_NAME, \ __func__, fmt, ##args); \ } while (0) - +#define DRM_VERB(fmt, args...) \ + do {\ + drm_ut_debug_printk(DRM_UT_CORE_VERB, DRM_NAME, \ + __func__, fmt, ##args); \ + } while (0) #define DRM_DEBUG_DRIVER(fmt, args...) \ do {\ drm_ut_debug_printk(DRM_UT_DRIVER, DRM_NAME,\ @@ -247,6 +253,7 @@ int drm_err(const char *func, const char *format, ...); #define DRM_DEBUG_KMS(fmt, args...)do { } while (0) #define DRM_DEBUG_PRIME(fmt, args...) do { } while (0) #define DRM_DEBUG(fmt, arg...) do { } while (0) +#define DRM_VERB(fmt, arg...) do { } while (0) #define DRM_LOG(fmt, arg...) do { } while (0) #define DRM_LOG_KMS(fmt, args...) do { } while (0) #define DRM_LOG_MODE(fmt, arg...) do { } while (0) -- 1.7.9.5
[RFC 0/4] drm: add raw monotonic timestamp support
On Fri, 2012-10-05 at 16:07 -0700, Eric Anholt wrote: > Imre Deak writes: > > > This is needed to make applications depending on vblank/page flip > > timestamps independent of time ajdustments. > > > > I've tested these with an updated intel-gpu-test/flip_test and will send > > the update for that once there's no objection about this patchset. > > > > The patchset is based on danvet's dinq branch with the following > > additional patches from the intel-gfx ML applied: > > drm/i915: paper over a pipe-enable vs pageflip race > > drm/i915: don't frob the vblank ts in finish_page_flip > > drm/i915: call drm_handle_vblank before finish_page_flip > > While people are in pageflip code: > > It would be really, really cool for application tracing if we could get > timestamps out of our swaps that used the TIMESTAMP register that is the > timer used for event tracing on the GPU using GL_ARB_timer_query. Then > you could get decent visualizations of the latency of your rendering. I assume this querying wouldn't be done through the wait_for_vblank or page_flip ioctls, but rather a new ioctl or even through a new perf event? We could add such a new interface later; this patchset is more focused on the above two ioctls. --Imre
[PATCH 2/2 v6] of: add generic videomode description
Hi Steffen, On Monday 08 October 2012 09:57:41 Steffen Trumtrar wrote: > On Mon, Oct 08, 2012 at 10:21:53AM +0300, Tomi Valkeinen wrote: > > On Thu, 2012-10-04 at 19:59 +0200, Steffen Trumtrar wrote: [snip] > > > diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h > > > new file mode 100644 > > > index 000..96efe01 > > > --- /dev/null > > > +++ b/include/linux/of_videomode.h > > > @@ -0,0 +1,41 @@ > > > +/* > > > + * Copyright 2012 Steffen Trumtrar > > > + * > > > + * generic videomode description > > > + * > > > + * This file is released under the GPLv2 > > > + */ > > > + > > > +#ifndef __LINUX_VIDEOMODE_H > > > +#define __LINUX_VIDEOMODE_H > > > + > > > +#include > > > > You don't need to include this. > > That is a fix to my liking. Easily done ;-) > > > > +struct videomode { > > > + u32 pixelclock; > > > + u32 refreshrate; > > > + > > > + u32 hactive; > > > + u32 hfront_porch; > > > + u32 hback_porch; > > > + u32 hsync_len; > > > + > > > + u32 vactive; > > > + u32 vfront_porch; > > > + u32 vback_porch; > > > + u32 vsync_len; > > > + > > > + bool hah; > > > + bool vah; > > > + bool interlaced; > > > + bool doublescan; > > > + > > > +}; > > > > This is not really of related. And actually, neither is the struct > > signal_timing in the previous patch. It would be nice to have these in a > > common header that fb, drm, and others could use instead of each having > > their own timing structs. > > > > But that's probably out of scope for this series =). Did you check the > > timing structs from the video related frameworks in the kernel to see if > > your structs contain all the info the others have, so that, at least in > > theory, everybody could use these common structs? > > > > Tomi > > Yes. Stephen and Laurent already suggested to split it up. > No, all info is not contained. That starts with drm, which has width-mm,.. > If time permits, I will go over that. Just to make sure we won't forget it, the V4L2 version of the timings structure is struct v4l2_bt_timings in include/linux/videodev2.h. -- Regards, Laurent Pinchart
[PATCH 2/2 v6] of: add generic videomode description
Hi Steffen, Thanks for the patch. On Thursday 04 October 2012 19:59:20 Steffen Trumtrar wrote: > Get videomode from devicetree in a format appropriate for the > backend. drm_display_mode and fb_videomode are supported atm. > Uses the display signal timings from of_display_timings > > Signed-off-by: Steffen Trumtrar > --- > drivers/of/Kconfig |5 + > drivers/of/Makefile |1 + > drivers/of/of_videomode.c| 212 +++ > include/linux/of_videomode.h | 41 > 4 files changed, 259 insertions(+) > create mode 100644 drivers/of/of_videomode.c > create mode 100644 include/linux/of_videomode.h > > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig > index 646deb0..74282e2 100644 > --- a/drivers/of/Kconfig > +++ b/drivers/of/Kconfig > @@ -88,4 +88,9 @@ config OF_DISPLAY_TIMINGS > help > helper to parse display timings from the devicetree > > +config OF_VIDEOMODE > + def_bool y > + help > + helper to get videomodes from the devicetree > + > endmenu # OF > diff --git a/drivers/of/Makefile b/drivers/of/Makefile > index c8e9603..09d556f 100644 > --- a/drivers/of/Makefile > +++ b/drivers/of/Makefile > @@ -12,3 +12,4 @@ obj-$(CONFIG_OF_PCI)+= of_pci.o > obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o > obj-$(CONFIG_OF_MTD) += of_mtd.o > obj-$(CONFIG_OF_DISPLAY_TIMINGS) += of_display_timings.o > +obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o > diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c > new file mode 100644 > index 000..76ac16e > --- /dev/null > +++ b/drivers/of/of_videomode.c > @@ -0,0 +1,212 @@ > +/* > + * generic videomode helper > + * > + * Copyright (c) 2012 Steffen Trumtrar , > Pengutronix > + * > + * This file is released under the GPLv2 > + */ > +#include > +#include > +#include > +#include > +#include > +#include > + > +void dump_fb_videomode(struct fb_videomode *m) > +{ > +pr_debug("fb_videomode = %d %d %d %d %d %d %d %d %d %d %d %d %d\n", That's going to be pretty difficult to read :-) Would it make sense to group several attributes logically (for instance using %ux%u for m->xres, m->yres) ? > +m->refresh, m->xres, m->yres, m->pixclock, m->left_margin, > +m->right_margin, m->upper_margin, m->lower_margin, + > m->hsync_len, m->vsync_len, m->sync, m->vmode, m->flag); > +} Shouldn't this (and the other non exported functions below) be static ? > +void dump_drm_displaymode(struct drm_display_mode *m) > +{ > +pr_debug("drm_displaymode = %d %d %d %d %d %d %d %d %d\n", > +m->hdisplay, m->hsync_start, m->hsync_end, m->htotal, > +m->vdisplay, m->vsync_start, m->vsync_end, m->vtotal, > +m->clock); > +} > + > +int videomode_from_timing(struct display_timings *disp, struct videomode > *vm, > + int index) > +{ > + struct signal_timing *st = NULL; > + > + if (!vm) > + return -EINVAL; > + What about making vm a mandatory argument ? It looks to me like a caller bug if vm is NULL. > + st = display_timings_get(disp, index); > + You can remove the blank line. > + if (!st) { > + pr_err("%s: no signal timings found\n", __func__); > + return -EINVAL; > + } > + > + vm->pixelclock = signal_timing_get_value(>pixelclock, 0); > + vm->hactive = signal_timing_get_value(>hactive, 0); > + vm->hfront_porch = signal_timing_get_value(>hfront_porch, 0); > + vm->hback_porch = signal_timing_get_value(>hback_porch, 0); > + vm->hsync_len = signal_timing_get_value(>hsync_len, 0); > + > + vm->vactive = signal_timing_get_value(>vactive, 0); > + vm->vfront_porch = signal_timing_get_value(>vfront_porch, 0); > + vm->vback_porch = signal_timing_get_value(>vback_porch, 0); > + vm->vsync_len = signal_timing_get_value(>vsync_len, 0); > + > + vm->vah = st->vsync_pol_active_high; > + vm->hah = st->hsync_pol_active_high; > + vm->interlaced = st->interlaced; > + vm->doublescan = st->doublescan; > + > + return 0; > +} > + > +int of_get_videomode(struct device_node *np, struct videomode *vm, int > index) I wonder how to avoid abuse of this functions. It's a useful helper for drivers that need to get a video mode once only, but would result in lower performances if a driver calls it for every mode. Drivers must call of_get_display_timing_list instead in that case and case the display timings. I'm wondering whether we should really expose of_get_videomode. > +{ > + struct display_timings *disp; > + int ret = 0; No need to assign ret to 0 here. > + > + disp = of_get_display_timing_list(np); > + You can remove the blank line. > + if (!disp) { > + pr_err("%s: no timings specified\n", __func__); > + return -EINVAL; > + } > + > + if (index == OF_DEFAULT_TIMING) > + index = disp->default_timing; > + > +
[PATCH 1/2 v6] of: add helper to parse display timings
Hi Tomi, On Monday 08 October 2012 12:01:18 Tomi Valkeinen wrote: > On Mon, 2012-10-08 at 10:25 +0200, Guennadi Liakhovetski wrote: > > In general, I might be misunderstanding something, but don't we have to > > distinguish between 2 types of information about display timings: (1) is > > defined by the display controller requirements, is known to the display > > driver and doesn't need to be present in timings DT. We did have some of > > these parameters in board data previously, because we didn't have proper > > display controller drivers... (2) is board specific configuration, and is > > such it has to be present in DT. > > > > In that way, doesn't "interlaced" belong to type (1) and thus doesn't need > > to be present in DT? > > As I see it, this DT data is about the display (most commonly LCD > panel), i.e. what video mode(s) the panel supports. If things were done > my way, the panel's supported timings would be defined in the driver for > the panel, and DT would be left to describe board specific data, but > this approach has its benefits. What about dumb DPI panels ? They will all be supported by a single driver, would you have the driver contain information about all known DPI panels ? DT seems a good solution to me in this case. For complex panels where the driver will support a single (or very few) model I agree that specifying the timings in DT isn't needed. > Thus, if you connect an interlaced panel to your board, you need to tell > the display controller that this panel requires interlace signal. Also, > pixel clock source doesn't make sense in this context, as this doesn't > describe the actual used configuration, but only what the panel > supports. > > Of course, if this is about describing the hardware, the default-mode > property doesn't really fit in... Maybe we should rename it to native-mode then ? -- Regards, Laurent Pinchart
[PATCH] gma500: medfield: fix potential NULL pointer dereference in mdfld_dsi_output_init()
On Sun, 7 Oct 2012 21:56:45 +0800 Wei Yongjun wrote: > From: Wei Yongjun > > The dereference should be moved below the NULL test. The !dev check just wants removing I think - it's a bogus check in the first place.
[PATCHv9 18/25] v4l: add buffer exporting via dmabuf
On Mon October 8 2012 12:41:28 Tomasz Stanislawski wrote: > Hi Hans, > > On 10/08/2012 11:54 AM, Hans Verkuil wrote: > > On Mon October 8 2012 11:40:37 Tomasz Stanislawski wrote: > >> Hi Hans, > >> > >> On 10/07/2012 04:17 PM, Hans Verkuil wrote: > >>> On Sun October 7 2012 15:38:30 Laurent Pinchart wrote: > Hi Hans, > > On Friday 05 October 2012 10:55:40 Hans Verkuil wrote: > > On Tue October 2 2012 16:27:29 Tomasz Stanislawski wrote: > >> This patch adds extension to V4L2 api. It allow to export a mmap > >> buffer as > >> file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer > >> offset used by mmap and return a file descriptor on success. > >> > >> Signed-off-by: Tomasz Stanislawski > >> Signed-off-by: Kyungmin Park > [snip] > >> +struct v4l2_exportbuffer { > >> + __s32 fd; > >> + __u32 flags; > >> + __u32 type; /* enum v4l2_buf_type */ > >> + __u32 index; > >> + __u32 plane; > > > > As suggested in my comments in the previous patch, I think it is a more > > natural order to have the type/index/plane fields first in this struct. > > > > Actually, I think that flags should also come before fd: > > > > struct v4l2_exportbuffer { > > __u32 type; /* enum v4l2_buf_type */ > > __u32 index; > > __u32 plane; > > __u32 flags; > > __s32 fd; > > __u32 reserved[11]; > > }; > > It would indeed feel more natural, but putting them right before the > reserved > fields allows creating an anonymous union around type, index and plane > and > extending it with reserved fields if needed. That's (at least to my > understanding) the rationale behind the current structure layout. > >>> > >>> The anonymous union argument makes no sense to me, to be honest. > >> > >> I agree that the anonymous unions are not good solutions because they are > >> not > >> supported in many C dialects. However I have nothing against using named > >> unions. > > > > Named or unnamed, I don't see how a union will help. What do you want to do > > with a union? > > > > Currently, there exist three sane layouts of the structure, that use > only one reserved field: > > A) > struct v4l2_exportbuffer { > __s32 fd; > __u32 flags; > __u32 type; /* enum v4l2_buf_type */ > __u32 index; > __u32 plane; > __u32 reserved[11]; > } > > B) > struct v4l2_exportbuffer { > __u32 type; /* enum v4l2_buf_type */ > __u32 index; > __u32 plane; > __u32 flags; > __s32 fd; > __u32 reserved[11]; > } > > C) > struct v4l2_exportbuffer { > __u32 type; /* enum v4l2_buf_type */ > __u32 index; > __u32 plane; > __u32 reserved[11]; > __u32 flags; > __s32 fd; > } > > Only the layout B follows 'input/output/reserved' rule. > > The layouts A and C allows to extend (type/index/plane) tuple without mixing > it with (flags,fd). > > For layouts A and C it is possible to use unions to provide new > means of describing a buffer to be exported. > > struct v4l2_exportbuffer { > __s32 fd; > __u32 flags; > union { > struct by_tip { /* type, index, plane */ > __u32 type; /* enum v4l2_buf_type */ > __u32 index; > __u32 plane; > } by_tip; > struct by_userptr { > u64 userptr; > u64 length; > } by_userptr; > __u32 reserved[6]; > } b; > __u32 union_type; /* BY_TIP or BY_USERPTR */ > __u32 reserved[4]; > }; > > No such an extension can be applied for layout B. You are overengineering. If we ever want to export something that is *not* a buffer created with REQBUFS/CREATE_BUFS, then you want to do that with a new ioctl. If we want for some reason to export a userptr, then that should either be from a queued/prepared buffer, or we need a separate API to create a dmabuf from a userptr. After all, that would be completely generic and not V4L2 specific. Anyway, introducing such a union later won't work either because then instead of writing expbuf.type you'd have to write expbuf.by_tip.type: API change. BTW, I think it makes sense as well in the case of a userptr to only export the buffer if it is under control of the kernel (e.g. after QBUF or PREPARE_BUF). When exporting the buffer the driver has all the information it needs to do so safely. > The similar scheme can be
[PATCHv9 18/25] v4l: add buffer exporting via dmabuf
Hi Hans, On 10/08/2012 11:54 AM, Hans Verkuil wrote: > On Mon October 8 2012 11:40:37 Tomasz Stanislawski wrote: >> Hi Hans, >> >> On 10/07/2012 04:17 PM, Hans Verkuil wrote: >>> On Sun October 7 2012 15:38:30 Laurent Pinchart wrote: Hi Hans, On Friday 05 October 2012 10:55:40 Hans Verkuil wrote: > On Tue October 2 2012 16:27:29 Tomasz Stanislawski wrote: >> This patch adds extension to V4L2 api. It allow to export a mmap buffer >> as >> file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer >> offset used by mmap and return a file descriptor on success. >> >> Signed-off-by: Tomasz Stanislawski >> Signed-off-by: Kyungmin Park [snip] >> +struct v4l2_exportbuffer { >> +__s32 fd; >> +__u32 flags; >> +__u32 type; /* enum v4l2_buf_type */ >> +__u32 index; >> +__u32 plane; > > As suggested in my comments in the previous patch, I think it is a more > natural order to have the type/index/plane fields first in this struct. > > Actually, I think that flags should also come before fd: > > struct v4l2_exportbuffer { > __u32 type; /* enum v4l2_buf_type */ > __u32 index; > __u32 plane; > __u32 flags; > __s32 fd; > __u32 reserved[11]; > }; It would indeed feel more natural, but putting them right before the reserved fields allows creating an anonymous union around type, index and plane and extending it with reserved fields if needed. That's (at least to my understanding) the rationale behind the current structure layout. >>> >>> The anonymous union argument makes no sense to me, to be honest. >> >> I agree that the anonymous unions are not good solutions because they are not >> supported in many C dialects. However I have nothing against using named >> unions. > > Named or unnamed, I don't see how a union will help. What do you want to do > with a union? > Currently, there exist three sane layouts of the structure, that use only one reserved field: A) struct v4l2_exportbuffer { __s32 fd; __u32 flags; __u32 type; /* enum v4l2_buf_type */ __u32 index; __u32 plane; __u32 reserved[11]; } B) struct v4l2_exportbuffer { __u32 type; /* enum v4l2_buf_type */ __u32 index; __u32 plane; __u32 flags; __s32 fd; __u32 reserved[11]; } C) struct v4l2_exportbuffer { __u32 type; /* enum v4l2_buf_type */ __u32 index; __u32 plane; __u32 reserved[11]; __u32 flags; __s32 fd; } Only the layout B follows 'input/output/reserved' rule. The layouts A and C allows to extend (type/index/plane) tuple without mixing it with (flags,fd). For layouts A and C it is possible to use unions to provide new means of describing a buffer to be exported. struct v4l2_exportbuffer { __s32 fd; __u32 flags; union { struct by_tip { /* type, index, plane */ __u32 type; /* enum v4l2_buf_type */ __u32 index; __u32 plane; } by_tip; struct by_userptr { u64 userptr; u64 length; } by_userptr; __u32 reserved[6]; } b; __u32 union_type; /* BY_TIP or BY_USERPTR */ __u32 reserved[4]; }; No such an extension can be applied for layout B. The similar scheme can be used for layout C. Moreover it support extensions and variants for (flags/fd) tuple. It might be useful if one day we would like to export a buffer as something different from DMABUF file descriptors. Anyway, we have to choose between the elegance of the layout and the extensibility. I think that layout A is a good trade-off. We could swap fd and flags to get little closer to "the rule". >> >>> It's standard practice within V4L2 to have the IN fields first, then the >>> OUT fields, followed >>> by reserved fields for future expansion. >> >> IMO, the "input/output/reserved rule" is only a recommendation. >> The are places in V4L2 where this rule is violated with structure >> v4l2_buffer being the most notable example. >> >> Notice that if at least one of the reserved fields becomes an input >> file then "the rule" will be violated anyway. > > Sure, but there is no legacy yet, so why not keep to the recommendation? > >>> Should we ever need a, say, sub-plane >>> index (whatever that might be), then we can use one of the reserved fields. >> >> Maybe
[PATCH] drm/radeon/cayman: set VM max pfn at MC init
From: Alex DeucherNo need to emit them at VM flush as we no longer use variable sized page tables now that we support 2 level page tables. This matches the behavior of SI (which does not support variable sized page tables). Signed-off-by: Alex Deucher --- drivers/gpu/drm/radeon/ni.c |8 +--- 1 files changed, 1 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c index cfb9276..cc0701a 100644 --- a/drivers/gpu/drm/radeon/ni.c +++ b/drivers/gpu/drm/radeon/ni.c @@ -776,7 +776,7 @@ static int cayman_pcie_gart_enable(struct radeon_device *rdev) */ for (i = 1; i < 8; i++) { WREG32(VM_CONTEXT0_PAGE_TABLE_START_ADDR + (i << 2), 0); - WREG32(VM_CONTEXT0_PAGE_TABLE_END_ADDR + (i << 2), 0); + WREG32(VM_CONTEXT0_PAGE_TABLE_END_ADDR + (i << 2), rdev->vm_manager.max_pfn); WREG32(VM_CONTEXT0_PAGE_TABLE_BASE_ADDR + (i << 2), rdev->gart.table_addr >> 12); } @@ -1576,12 +1576,6 @@ void cayman_vm_flush(struct radeon_device *rdev, int ridx, struct radeon_vm *vm) if (vm == NULL) return; - radeon_ring_write(ring, PACKET0(VM_CONTEXT0_PAGE_TABLE_START_ADDR + (vm->id << 2), 0)); - radeon_ring_write(ring, 0); - - radeon_ring_write(ring, PACKET0(VM_CONTEXT0_PAGE_TABLE_END_ADDR + (vm->id << 2), 0)); - radeon_ring_write(ring, rdev->vm_manager.max_pfn); - radeon_ring_write(ring, PACKET0(VM_CONTEXT0_PAGE_TABLE_BASE_ADDR + (vm->id << 2), 0)); radeon_ring_write(ring, vm->pd_gpu_addr >> 12); -- 1.7.7.5
[PATCH 1/3] drm/radeon: allocate page tables on demand v4
On Mon, Oct 8, 2012 at 11:55 AM, Christian K?nig wrote: > Based on Dmitries work, but splitting the code into page > directory and page table handling makes it far more > readable and (hopefully) more reliable. > > Allocations of page tables are made from the SA on demand, > that should still work fine since all page tables are of > the same size. > > Also using the fact that allocations from the SA are mostly > continuously (except for end of buffer wraps and under very > high memory pressure) to group updates send to the chipset > specific code into larger chunks. > > v3: mostly a rewrite of Dmitries previous patch. > v4: fix some typos and coding style > > Signed-off-by: Dmitry Cherkasov > Signed-off-by: Christian K?nig > Tested-by: Michel D?nzer For the series: Reviewed-by: Alex Deucher > --- > drivers/gpu/drm/radeon/ni.c |2 +- > drivers/gpu/drm/radeon/radeon.h | 11 +- > drivers/gpu/drm/radeon/radeon_gart.c | 322 > ++ > 3 files changed, 262 insertions(+), 73 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c > index 9a46f7d..48e2337 100644 > --- a/drivers/gpu/drm/radeon/ni.c > +++ b/drivers/gpu/drm/radeon/ni.c > @@ -1576,7 +1576,7 @@ void cayman_vm_flush(struct radeon_device *rdev, int > ridx, struct radeon_vm *vm) > radeon_ring_write(ring, 0); > > radeon_ring_write(ring, PACKET0(VM_CONTEXT0_PAGE_TABLE_END_ADDR + > (vm->id << 2), 0)); > - radeon_ring_write(ring, vm->last_pfn); > + radeon_ring_write(ring, rdev->vm_manager.max_pfn); > > radeon_ring_write(ring, PACKET0(VM_CONTEXT0_PAGE_TABLE_BASE_ADDR + > (vm->id << 2), 0)); > radeon_ring_write(ring, vm->pd_gpu_addr >> 12); > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h > index b04c064..bc6b56b 100644 > --- a/drivers/gpu/drm/radeon/radeon.h > +++ b/drivers/gpu/drm/radeon/radeon.h > @@ -663,9 +663,14 @@ struct radeon_vm { > struct list_headlist; > struct list_headva; > unsignedid; > - unsignedlast_pfn; > - u64 pd_gpu_addr; > - struct radeon_sa_bo *sa_bo; > + > + /* contains the page directory */ > + struct radeon_sa_bo *page_directory; > + uint64_tpd_gpu_addr; > + > + /* array of page tables, one for each page directory entry */ > + struct radeon_sa_bo **page_tables; > + > struct mutexmutex; > /* last fence for cs using this vm */ > struct radeon_fence *fence; > diff --git a/drivers/gpu/drm/radeon/radeon_gart.c > b/drivers/gpu/drm/radeon/radeon_gart.c > index 753b7ca..b36b615 100644 > --- a/drivers/gpu/drm/radeon/radeon_gart.c > +++ b/drivers/gpu/drm/radeon/radeon_gart.c > @@ -423,6 +423,18 @@ void radeon_gart_fini(struct radeon_device *rdev) > */ > > /** > + * radeon_vm_num_pde - return the number of page directory entries > + * > + * @rdev: radeon_device pointer > + * > + * Calculate the number of page directory entries (cayman+). > + */ > +static unsigned radeon_vm_num_pdes(struct radeon_device *rdev) > +{ > + return rdev->vm_manager.max_pfn >> RADEON_VM_BLOCK_SIZE; > +} > + > +/** > * radeon_vm_directory_size - returns the size of the page directory in bytes > * > * @rdev: radeon_device pointer > @@ -431,7 +443,7 @@ void radeon_gart_fini(struct radeon_device *rdev) > */ > static unsigned radeon_vm_directory_size(struct radeon_device *rdev) > { > - return (rdev->vm_manager.max_pfn >> RADEON_VM_BLOCK_SIZE) * 8; > + return RADEON_GPU_PAGE_ALIGN(radeon_vm_num_pdes(rdev) * 8); > } > > /** > @@ -451,11 +463,11 @@ int radeon_vm_manager_init(struct radeon_device *rdev) > > if (!rdev->vm_manager.enabled) { > /* allocate enough for 2 full VM pts */ > - size = RADEON_GPU_PAGE_ALIGN(radeon_vm_directory_size(rdev)); > - size += RADEON_GPU_PAGE_ALIGN(rdev->vm_manager.max_pfn * 8); > + size = radeon_vm_directory_size(rdev); > + size += rdev->vm_manager.max_pfn * 8; > size *= 2; > r = radeon_sa_bo_manager_init(rdev, > >vm_manager.sa_manager, > - size, > + RADEON_GPU_PAGE_ALIGN(size), > RADEON_GEM_DOMAIN_VRAM); > if (r) { > dev_err(rdev->dev, "failed to allocate vm bo > (%dKB)\n", > @@ -476,7 +488,7 @@ int radeon_vm_manager_init(struct radeon_device *rdev) > > /* restore page table */ > list_for_each_entry(vm, >vm_manager.lru_vm, list) { > - if (vm->sa_bo == NULL) > + if (vm->page_directory == NULL) > continue; > >
[PATCH] Android.mk: use LOCAL_COPY_HEADERS to export headers. v2
From: Haitao HuangExport necessary header files used by other components for Android, such as libva intel-driver, gralloc, hwcomposer, etc. Signed-off-by: Haitao Huang [chad: Fixed inconsistent indentation.] Signed-off-by: Chad Versace --- Android.mk | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Android.mk b/Android.mk index 1029dc6..68db5df 100644 --- a/Android.mk +++ b/Android.mk @@ -1,5 +1,5 @@ # -# Copyright ? 2011 Intel Corporation +# Copyright ? 2011-2012 Intel Corporation # # Permission is hereby granted, free of charge, to any person obtaining a # copy of this software and associated documentation files (the "Software"), @@ -45,6 +45,16 @@ LOCAL_C_INCLUDES := \ LOCAL_CFLAGS := \ -DHAVE_LIBDRM_ATOMIC_PRIMITIVES=1 +LOCAL_COPY_HEADERS :=\ + xf86drm.h\ + include/drm/drm_fourcc.h \ + include/drm/drm.h\ + include/drm/drm_mode.h \ + include/drm/drm_sarea.h \ + include/drm/i915_drm.h \ + intel/intel_bufmgr.h \ + +LOCAL_COPY_HEADERS_TO := libdrm include $(BUILD_SHARED_LIBRARY) include $(LOCAL_PATH)/intel/Android.mk -- 1.7.11.7
[PATCH 1/2] drm/radeon: update comments to clarify VM setup (v2)
From: Alex DeucherThe actual set up and assignment of VM page tables is done on the fly in radeon_gart.c. v2: update vm size comments Signed-off-by: Alex Deucher Reviewed-by: Christian K?nig --- drivers/gpu/drm/radeon/ni.c|4 drivers/gpu/drm/radeon/radeon_device.c |4 drivers/gpu/drm/radeon/si.c|7 --- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c index 9a46f7d..6883e14 100644 --- a/drivers/gpu/drm/radeon/ni.c +++ b/drivers/gpu/drm/radeon/ni.c @@ -770,6 +770,10 @@ static int cayman_pcie_gart_enable(struct radeon_device *rdev) WREG32(0x15DC, 0); /* empty context1-7 */ + /* Assign the pt base to something valid for now; the pts used for +* the VMs are determined by the application and setup and assigned +* on the fly in the vm part of radeon_gart.c +*/ for (i = 1; i < 8; i++) { WREG32(VM_CONTEXT0_PAGE_TABLE_START_ADDR + (i << 2), 0); WREG32(VM_CONTEXT0_PAGE_TABLE_END_ADDR + (i << 2), 0); diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 64a4264..bd13ca0 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -1018,6 +1018,10 @@ int radeon_device_init(struct radeon_device *rdev, return r; /* initialize vm here */ mutex_init(>vm_manager.lock); + /* Adjust VM size here. +* Currently set to 4GB ((1 << 20) 4k pages). +* Max GPUVM size for cayman and SI is 40 bits. +*/ rdev->vm_manager.max_pfn = 1 << 20; INIT_LIST_HEAD(>vm_manager.lru_vm); diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c index c76825f..f272ead 100644 --- a/drivers/gpu/drm/radeon/si.c +++ b/drivers/gpu/drm/radeon/si.c @@ -2407,12 +2407,13 @@ static int si_pcie_gart_enable(struct radeon_device *rdev) WREG32(0x15DC, 0); /* empty context1-15 */ - /* FIXME start with 4G, once using 2 level pt switch to full -* vm size space -*/ /* set vm size, must be a multiple of 4 */ WREG32(VM_CONTEXT1_PAGE_TABLE_START_ADDR, 0); WREG32(VM_CONTEXT1_PAGE_TABLE_END_ADDR, rdev->vm_manager.max_pfn); + /* Assign the pt base to something valid for now; the pts used for +* the VMs are determined by the application and setup and assigned +* on the fly in the vm part of radeon_gart.c +*/ for (i = 1; i < 16; i++) { if (i < 8) WREG32(VM_CONTEXT0_PAGE_TABLE_BASE_ADDR + (i << 2), -- 1.7.7.5
[PATCH 1/2 v6] of: add helper to parse display timings
On Mon, 2012-10-08 at 10:25 +0200, Guennadi Liakhovetski wrote: > In general, I might be misunderstanding something, but don't we have to > distinguish between 2 types of information about display timings: (1) is > defined by the display controller requirements, is known to the display > driver and doesn't need to be present in timings DT. We did have some of > these parameters in board data previously, because we didn't have proper > display controller drivers... (2) is board specific configuration, and is > such it has to be present in DT. > > In that way, doesn't "interlaced" belong to type (1) and thus doesn't need > to be present in DT? As I see it, this DT data is about the display (most commonly LCD panel), i.e. what video mode(s) the panel supports. If things were done my way, the panel's supported timings would be defined in the driver for the panel, and DT would be left to describe board specific data, but this approach has its benefits. Thus, if you connect an interlaced panel to your board, you need to tell the display controller that this panel requires interlace signal. Also, pixel clock source doesn't make sense in this context, as this doesn't describe the actual used configuration, but only what the panel supports. Of course, if this is about describing the hardware, the default-mode property doesn't really fit in... Tomi -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121008/6da1203e/attachment.pgp>
[PATCH] drm/radeon: allocate page tables on demand v3
Got time today to test that a bit more, no piglit regressions on SI so far. And also CAYMAN still seems to work with it, but there are allot of piglit tests there that still doesn't work when enabling VM (and it doesn't matter if you have this patch or not). Any objections to pull that into 3.7 as well? Cheers, Christian. On 07.10.2012 15:06, Christian K?nig wrote: > Based on Dmitries work, but splitting the code into page > directory and page table handling makes it far more > readable and (hopefully) more reliable. > > Allocations of page tables are made from the SA on demand, > that should still work fine since all page tables are of > the same size. > > Also using the fact that allocations from the SA are mostly > continuously (except for end of buffer wraps and under very > high memory pressure) to group updates send to the chipset > specific code into larger chunks. > > v3: mostly a rewrite of Dmitries previous patch. > > Signed-off-by: Dmitry Cherkasov > Signed-off-by: Christian K?nig > --- > drivers/gpu/drm/radeon/ni.c |2 +- > drivers/gpu/drm/radeon/radeon.h | 11 +- > drivers/gpu/drm/radeon/radeon_gart.c | 323 > ++ > 3 files changed, 263 insertions(+), 73 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c > index 9a46f7d..48e2337 100644 > --- a/drivers/gpu/drm/radeon/ni.c > +++ b/drivers/gpu/drm/radeon/ni.c > @@ -1576,7 +1576,7 @@ void cayman_vm_flush(struct radeon_device *rdev, int > ridx, struct radeon_vm *vm) > radeon_ring_write(ring, 0); > > radeon_ring_write(ring, PACKET0(VM_CONTEXT0_PAGE_TABLE_END_ADDR + > (vm->id << 2), 0)); > - radeon_ring_write(ring, vm->last_pfn); > + radeon_ring_write(ring, rdev->vm_manager.max_pfn); > > radeon_ring_write(ring, PACKET0(VM_CONTEXT0_PAGE_TABLE_BASE_ADDR + > (vm->id << 2), 0)); > radeon_ring_write(ring, vm->pd_gpu_addr >> 12); > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h > index b04c064..bc6b56b 100644 > --- a/drivers/gpu/drm/radeon/radeon.h > +++ b/drivers/gpu/drm/radeon/radeon.h > @@ -663,9 +663,14 @@ struct radeon_vm { > struct list_headlist; > struct list_headva; > unsignedid; > - unsignedlast_pfn; > - u64 pd_gpu_addr; > - struct radeon_sa_bo *sa_bo; > + > + /* contains the page directory */ > + struct radeon_sa_bo *page_directory; > + uint64_tpd_gpu_addr; > + > + /* array of page tables, one for each page directory entry */ > + struct radeon_sa_bo **page_tables; > + > struct mutexmutex; > /* last fence for cs using this vm */ > struct radeon_fence *fence; > diff --git a/drivers/gpu/drm/radeon/radeon_gart.c > b/drivers/gpu/drm/radeon/radeon_gart.c > index 753b7ca..40088b0 100644 > --- a/drivers/gpu/drm/radeon/radeon_gart.c > +++ b/drivers/gpu/drm/radeon/radeon_gart.c > @@ -423,6 +423,18 @@ void radeon_gart_fini(struct radeon_device *rdev) >*/ > > /** > + * radeon_vm_num_pde - return the number of page directory entries > + * > + * @rdev: radeon_device pointer > + * > + * Calculate the number of page directory entries (cayman+). > + */ > +static unsigned radeon_vm_num_pdes(struct radeon_device *rdev) > +{ > + return (rdev->vm_manager.max_pfn >> RADEON_VM_BLOCK_SIZE); > +} > + > +/** >* radeon_vm_directory_size - returns the size of the page directory in > bytes >* >* @rdev: radeon_device pointer > @@ -431,7 +443,7 @@ void radeon_gart_fini(struct radeon_device *rdev) >*/ > static unsigned radeon_vm_directory_size(struct radeon_device *rdev) > { > - return (rdev->vm_manager.max_pfn >> RADEON_VM_BLOCK_SIZE) * 8; > + return RADEON_GPU_PAGE_ALIGN(radeon_vm_num_pdes(rdev) * 8); > } > > /** > @@ -451,11 +463,11 @@ int radeon_vm_manager_init(struct radeon_device *rdev) > > if (!rdev->vm_manager.enabled) { > /* allocate enough for 2 full VM pts */ > - size = RADEON_GPU_PAGE_ALIGN(radeon_vm_directory_size(rdev)); > - size += RADEON_GPU_PAGE_ALIGN(rdev->vm_manager.max_pfn * 8); > + size = radeon_vm_directory_size(rdev); > + size += rdev->vm_manager.max_pfn * 8; > size *= 2; > r = radeon_sa_bo_manager_init(rdev, > >vm_manager.sa_manager, > - size, > + RADEON_GPU_PAGE_ALIGN(size), > RADEON_GEM_DOMAIN_VRAM); > if (r) { > dev_err(rdev->dev, "failed to allocate vm bo (%dKB)\n", > @@ -476,7 +488,7 @@ int radeon_vm_manager_init(struct radeon_device *rdev) > > /* restore page table */ >
[PATCHv9 18/25] v4l: add buffer exporting via dmabuf
On Mon October 8 2012 11:40:37 Tomasz Stanislawski wrote: > Hi Hans, > > On 10/07/2012 04:17 PM, Hans Verkuil wrote: > > On Sun October 7 2012 15:38:30 Laurent Pinchart wrote: > >> Hi Hans, > >> > >> On Friday 05 October 2012 10:55:40 Hans Verkuil wrote: > >>> On Tue October 2 2012 16:27:29 Tomasz Stanislawski wrote: > This patch adds extension to V4L2 api. It allow to export a mmap buffer > as > file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer > offset used by mmap and return a file descriptor on success. > > Signed-off-by: Tomasz Stanislawski > Signed-off-by: Kyungmin Park > >> > >> [snip] > >> > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h > index e04a73e..f429b6a 100644 > --- a/include/linux/videodev2.h > +++ b/include/linux/videodev2.h > @@ -688,6 +688,33 @@ struct v4l2_buffer { > > #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x0800 > #define V4L2_BUF_FLAG_NO_CACHE_CLEAN0x1000 > > +/** > + * struct v4l2_exportbuffer - export of video buffer as DMABUF file > descriptor + * > + * @fd: file descriptor associated with DMABUF (set by driver) > + * @flags: flags for newly created file, currently only O_CLOEXEC > is > + * supported, refer to manual of open syscall for more > details > + * @index: id number of the buffer > + * @type: enum v4l2_buf_type; buffer type (type == *_MPLANE for > + * multiplanar buffers); > + * @plane: index of the plane to be exported, 0 for single plane > queues > + * > + * Contains data used for exporting a video buffer as DMABUF file > descriptor. + * The buffer is identified by a 'cookie' returned by > VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to > userspace). All + * reserved fields must be set to zero. The field > reserved0 is expected to + * become a structure 'type' allowing an > alternative layout of the structure + * content. Therefore this field > should not be used for any other extensions. + */ > +struct v4l2_exportbuffer { > +__s32 fd; > +__u32 flags; > +__u32 type; /* enum v4l2_buf_type */ > +__u32 index; > +__u32 plane; > >>> > >>> As suggested in my comments in the previous patch, I think it is a more > >>> natural order to have the type/index/plane fields first in this struct. > >>> > >>> Actually, I think that flags should also come before fd: > >>> > >>> struct v4l2_exportbuffer { > >>> __u32 type; /* enum v4l2_buf_type */ > >>> __u32 index; > >>> __u32 plane; > >>> __u32 flags; > >>> __s32 fd; > >>> __u32 reserved[11]; > >>> }; > >> > >> It would indeed feel more natural, but putting them right before the > >> reserved > >> fields allows creating an anonymous union around type, index and plane and > >> extending it with reserved fields if needed. That's (at least to my > >> understanding) the rationale behind the current structure layout. > > > > The anonymous union argument makes no sense to me, to be honest. > > I agree that the anonymous unions are not good solutions because they are not > supported in many C dialects. However I have nothing against using named > unions. Named or unnamed, I don't see how a union will help. What do you want to do with a union? > > > It's standard practice within V4L2 to have the IN fields first, then the > > OUT fields, followed > > by reserved fields for future expansion. > > IMO, the "input/output/reserved rule" is only a recommendation. > The are places in V4L2 where this rule is violated with structure > v4l2_buffer being the most notable example. > > Notice that if at least one of the reserved fields becomes an input > file then "the rule" will be violated anyway. Sure, but there is no legacy yet, so why not keep to the recommendation? > > Should we ever need a, say, sub-plane > > index (whatever that might be), then we can use one of the reserved fields. > > Maybe not subplane :). > But maybe some data_offset for exporting only a part of the buffer will > be needed some day. > Moreover, the integration of DMABUF with the DMA synchronization framework > may involve passing additional parameters from the userspace. > > Notice that flags and fd fields are not logically connected with > (type/index/plane) tuple. > Therefore both field sets should be separated by some reserved fields to > ensure that any of them can be extended if needed. > > This was the rationale for the structure layout in v9. It's a bad idea to add multiple 'reserved' arrays, that makes userspace harder since it has to zero all of them instead of just one. Actually, the same applies to kernel space, which has to zero
[PATCH] drm/radeon: update comments to clarify VM setup
On Mon, Oct 8, 2012 at 11:29 AM, Christian K?nig wrote: > On 08.10.2012 15:50, alexdeucher at gmail.com wrote: >> >> From: Alex Deucher >> >> The actual set up and assignment of VM page tables >> is done on the fly in radeon_gart.c. >> >> Signed-off-by: Alex Deucher > > One comment below, apart from that it's: > > Reviewed-by: Christian K?nig > > >> --- >> drivers/gpu/drm/radeon/ni.c|4 >> drivers/gpu/drm/radeon/radeon_device.c |3 +++ >> drivers/gpu/drm/radeon/si.c|7 --- >> 3 files changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c >> index 48e2337..cfb9276 100644 >> --- a/drivers/gpu/drm/radeon/ni.c >> +++ b/drivers/gpu/drm/radeon/ni.c >> @@ -770,6 +770,10 @@ static int cayman_pcie_gart_enable(struct >> radeon_device *rdev) >> WREG32(0x15DC, 0); >> /* empty context1-7 */ >> + /* Assign the pt base to something valid for now; the pts used for >> +* the VMs are determined by the application and setup and >> assigned >> +* on the fly in the vm part of radeon_gart.c >> +*/ >> for (i = 1; i < 8; i++) { >> WREG32(VM_CONTEXT0_PAGE_TABLE_START_ADDR + (i << 2), 0); >> WREG32(VM_CONTEXT0_PAGE_TABLE_END_ADDR + (i << 2), 0); >> diff --git a/drivers/gpu/drm/radeon/radeon_device.c >> b/drivers/gpu/drm/radeon/radeon_device.c >> index 64a4264..1fad47e 100644 >> --- a/drivers/gpu/drm/radeon/radeon_device.c >> +++ b/drivers/gpu/drm/radeon/radeon_device.c >> @@ -1018,6 +1018,9 @@ int radeon_device_init(struct radeon_device *rdev, >> return r; >> /* initialize vm here */ >> mutex_init(>vm_manager.lock); >> + /* FIXME start with 4G, once using 2 level pt switch to full >> +* vm size space >> +*/ > > Even with 2 level pt I don't think we want to switch to the full vm size > space. > > Having 40bits address space minus 12bits offsets gives us 28bits for the > page directory/page table, assuming a 50/50 split between them we would get > 2 ^ 14 * 8 = 128kb for each page, which in my eyes is a bit much (and that > gets even worth when we get 48bits address space.) > > I would rather say to make max_pfn depend on the available memory, so that a > single application can at least map all VRAM + GART memory. I'll make the comment a bit clearer. I was mostly just moving the comment to be near the code that sets that value. Alex > > >> rdev->vm_manager.max_pfn = 1 << 20; >> INIT_LIST_HEAD(>vm_manager.lru_vm); >> diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c >> index c76825f..f272ead 100644 >> --- a/drivers/gpu/drm/radeon/si.c >> +++ b/drivers/gpu/drm/radeon/si.c >> @@ -2407,12 +2407,13 @@ static int si_pcie_gart_enable(struct >> radeon_device *rdev) >> WREG32(0x15DC, 0); >> /* empty context1-15 */ >> - /* FIXME start with 4G, once using 2 level pt switch to full >> -* vm size space >> -*/ >> /* set vm size, must be a multiple of 4 */ >> WREG32(VM_CONTEXT1_PAGE_TABLE_START_ADDR, 0); >> WREG32(VM_CONTEXT1_PAGE_TABLE_END_ADDR, rdev->vm_manager.max_pfn); >> + /* Assign the pt base to something valid for now; the pts used for >> +* the VMs are determined by the application and setup and >> assigned >> +* on the fly in the vm part of radeon_gart.c >> +*/ >> for (i = 1; i < 16; i++) { >> if (i < 8) >> WREG32(VM_CONTEXT0_PAGE_TABLE_BASE_ADDR + (i << >> 2), > >
[PATCHv9 18/25] v4l: add buffer exporting via dmabuf
Hi Hans, On 10/07/2012 04:17 PM, Hans Verkuil wrote: > On Sun October 7 2012 15:38:30 Laurent Pinchart wrote: >> Hi Hans, >> >> On Friday 05 October 2012 10:55:40 Hans Verkuil wrote: >>> On Tue October 2 2012 16:27:29 Tomasz Stanislawski wrote: This patch adds extension to V4L2 api. It allow to export a mmap buffer as file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by mmap and return a file descriptor on success. Signed-off-by: Tomasz Stanislawski Signed-off-by: Kyungmin Park >> >> [snip] >> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index e04a73e..f429b6a 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -688,6 +688,33 @@ struct v4l2_buffer { #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x0800 #define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x1000 +/** + * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor + * + * @fd: file descriptor associated with DMABUF (set by driver) + * @flags:flags for newly created file, currently only O_CLOEXEC is + *supported, refer to manual of open syscall for more details + * @index:id number of the buffer + * @type: enum v4l2_buf_type; buffer type (type == *_MPLANE for + *multiplanar buffers); + * @plane:index of the plane to be exported, 0 for single plane queues + * + * Contains data used for exporting a video buffer as DMABUF file descriptor. + * The buffer is identified by a 'cookie' returned by VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to userspace). All + * reserved fields must be set to zero. The field reserved0 is expected to + * become a structure 'type' allowing an alternative layout of the structure + * content. Therefore this field should not be used for any other extensions. + */ +struct v4l2_exportbuffer { + __s32 fd; + __u32 flags; + __u32 type; /* enum v4l2_buf_type */ + __u32 index; + __u32 plane; >>> >>> As suggested in my comments in the previous patch, I think it is a more >>> natural order to have the type/index/plane fields first in this struct. >>> >>> Actually, I think that flags should also come before fd: >>> >>> struct v4l2_exportbuffer { >>> __u32 type; /* enum v4l2_buf_type */ >>> __u32 index; >>> __u32 plane; >>> __u32 flags; >>> __s32 fd; >>> __u32 reserved[11]; >>> }; >> >> It would indeed feel more natural, but putting them right before the >> reserved >> fields allows creating an anonymous union around type, index and plane and >> extending it with reserved fields if needed. That's (at least to my >> understanding) the rationale behind the current structure layout. > > The anonymous union argument makes no sense to me, to be honest. I agree that the anonymous unions are not good solutions because they are not supported in many C dialects. However I have nothing against using named unions. > It's standard practice within V4L2 to have the IN fields first, then the OUT > fields, followed > by reserved fields for future expansion. IMO, the "input/output/reserved rule" is only a recommendation. The are places in V4L2 where this rule is violated with structure v4l2_buffer being the most notable example. Notice that if at least one of the reserved fields becomes an input file then "the rule" will be violated anyway. > Should we ever need a, say, sub-plane > index (whatever that might be), then we can use one of the reserved fields. Maybe not subplane :). But maybe some data_offset for exporting only a part of the buffer will be needed some day. Moreover, the integration of DMABUF with the DMA synchronization framework may involve passing additional parameters from the userspace. Notice that flags and fd fields are not logically connected with (type/index/plane) tuple. Therefore both field sets should be separated by some reserved fields to ensure that any of them can be extended if needed. This was the rationale for the structure layout in v9. Regards, Tomasz Stanislawski > Regards, > > Hans > >> + __u32 reserved[11]; +}; + /* *O V E R L A Y P R E V I E W */ >> >> >
Using Linux 3.2.23, GNOME 3.4 fallback, `evolution --component=mail mailto:i...@example.net` hangs
On Son, 2012-10-07 at 14:50 +0200, Paul Menzel wrote: > > using Debian Sid/unstable, clicking on a mail address in Iceweasel > (Firefox) a composer window is opened in Evolution 3.4.3-1. But then the > windows are only gray and hang and the only way to fix this is to kill > the process. I reported this issue to the GNOME Bugzilla as 685640 and > the developer Matthew Barnes pointed out the following [1]. > > --- Comment #1 from Matthew Barnes 2012-10-07 > 11:59:35 UTC --- > Radeon video driver thread looks stuck: > > Thread 5 (Thread 0x7f6656d4d700 (LWP 4375)): > #0 pthread_cond_wait@@GLIBC_2.3.2 () > at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:162 > No locals. > #1 0x7f6656f4c9f3 in pipe_semaphore_wait (sema=0x7f666b6577b8) > at ../../../../../src/gallium/auxiliary/os/os_thread.h:432 > No locals. > #2 radeon_drm_cs_emit_ioctl (param=0x7f666b635c40) at > radeon_drm_cs.c:401 > cs = 0x7f666b635c40 I followed up on the bug report with an explanation why that isn't necessarily a problem. -- Earthling Michel D?nzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 190 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121008/32223297/attachment.pgp>
[PATCH 1/2 v6] of: add helper to parse display timings
On Fri, 5 Oct 2012, Stephen Warren wrote: > On 10/04/2012 03:35 PM, Guennadi Liakhovetski wrote: > > Hi Steffen > > > > Sorry for chiming in so late in the game, but I've long been wanting to > > have a look at this and compare with what we do for V4L2, so, this seems a > > great opportunity to me:-) > > > > On Thu, 4 Oct 2012, Steffen Trumtrar wrote: > > >> diff --git a/Documentation/devicetree/bindings/video/display-timings.txt > >> b/Documentation/devicetree/bindings/video/display-timings.txt > > >> +timings-subnode > >> +--- > >> + > >> +required properties: > >> + - hactive, vactive: Display resolution > >> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing > >> parameters > >> + in pixels > >> + vfront-porch, vback-porch, vsync-len: Vertical display timing > >> parameters in > >> + lines > >> + - clock: displayclock in Hz > > > > You're going to hate me for this, but eventually we want to actually > > reference clock objects in our DT bindings. For now, even if you don't > > want to actually add clock phandles and stuff here, I think, using the > > standard "clock-frequency" property would be much better! > > In a definition of a display timing, we will never need to use the clock > binding; the clock binding would be used by the HW module that is > generating a timing, not by the timing definition itself. You mean clock consumer bindings will be in the display device DT node? And the display-timings node will be its child? > That said, your comment about renaming the property to avoid any kind of > conceptual conflict is still quite valid. This is bike-shedding, but > "pixel-clock" might be more in line with typical video mode terminology, > although there's certainly preference in DT for using the generic term > clock-frequency that you proposed. Either is fine by me. > > >> +optional properties: > >> + - hsync-active-high (bool): Hsync pulse is active high > >> + - vsync-active-high (bool): Vsync pulse is active high > > > > For the above two we also considered using bool properties but eventually > > settled down with integer ones: > > > > - hsync-active = <1> > > > > for active-high and 0 for active low. This has the added advantage of > > being able to omit this property in the .dts, which then doesn't mean, > > that the polarity is active low, but rather, that the hsync line is not > > used on this hardware. So, maybe it would be good to use the same binding > > here too? > > I agree. This also covers the case where analog display connectors often > use polarity to differentiate similar modes, yet digital connectors > often always use a fixed polarity since the receiving device can > "measure" the signal in more complete ways. > > If the board HW inverts these lines, the same property names can exist > in the display controller itself, and the two values XORd together to > yield the final output polarity. > > >> + - de-active-high (bool): Data-Enable pulse is active high > >> + - pixelclk-inverted (bool): pixelclock is inverted > > > > We don't (yet) have a de-active property in V4L, don't know whether we'll > > ever have to distingsuish between what some datasheets call "HREF" and > > HSYNC in DT, but maybe similarly to the above an integer would be > > preferred. As for pixclk, we call the property "pclk-sample" and it's also > > an integer. > > Thinking about this more: de-active-high is likely to be a > board-specific property and hence something in the display controller, > not in the mode definition? > > >> + - interlaced (bool) > > > > Is "interlaced" a property of the hardware, i.e. of the board? Can the > > same display controller on one board require interlaced data and on > > another board - progressive? > > Interlace is a property of a display mode. It's quite possible for a > particular display controller to switch between interlace and > progressive output at run-time. For example, reconfiguring the output > between 480i, 720p, 1080i, 1080p modes. Admittedly, if you're talking to > a built-in LCD display, you're probably always going to be driving the > single mode required by the panel, and that mode will likely always be > progressive. However, since this binding attempts to describe any > display timing, I think we still need this property per mode. But why do you need this in the DT then at all? If it's fixed, as required per display controller, then its driver will know it. If it's runtime configurable, then it's a purely software parameter and doesn't depend on the board? > > BTW, I'm not very familiar with display > > interfaces, but for interlaced you probably sometimes use a field signal, > > whose polarity you also want to specify here? We use a "field-even-active" > > integer property for it. > > I think that's a property of the display controller itself, rather than > an individual mode, although I'm not 100% certain. My assertion is that > the physical interface that the display
[PATCH 2/2 v6] of: add generic videomode description
__func__, index); > + return -EINVAL; > + } > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(of_get_videomode); > + > +#if defined(CONFIG_DRM) > +int videomode_to_display_mode(struct videomode *vm, struct drm_display_mode > *dmode) > +{ > + memset(dmode, 0, sizeof(*dmode)); > + > + dmode->hdisplay = vm->hactive; > + dmode->hsync_start = dmode->hdisplay + vm->hfront_porch; > + dmode->hsync_end = dmode->hsync_start + vm->hsync_len; > + dmode->htotal = dmode->hsync_end + vm->hback_porch; > + > + dmode->vdisplay = vm->vactive; > + dmode->vsync_start = dmode->vdisplay + vm->vfront_porch; > + dmode->vsync_end = dmode->vsync_start + vm->vsync_len; > + dmode->vtotal = dmode->vsync_end + vm->vback_porch; > + > + dmode->clock = vm->pixelclock / 1000; > + > + if (vm->hah) > + dmode->flags |= DRM_MODE_FLAG_PHSYNC; > + else > + dmode->flags |= DRM_MODE_FLAG_NHSYNC; > + if (vm->vah) > + dmode->flags |= DRM_MODE_FLAG_PVSYNC; > + else > + dmode->flags |= DRM_MODE_FLAG_NVSYNC; > + if (vm->interlaced) > + dmode->flags |= DRM_MODE_FLAG_INTERLACE; > + if (vm->doublescan) > + dmode->flags |= DRM_MODE_FLAG_DBLSCAN; > + drm_mode_set_name(dmode); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(videomode_to_display_mode); > + > +int of_get_drm_display_mode(struct device_node *np, struct drm_display_mode > *dmode, > + int index) > +{ > + struct videomode vm; > + int ret; > + > + ret = of_get_videomode(np, , index); > + > + if (ret) > + return ret; > + > + videomode_to_display_mode(, dmode); > + > + pr_info("%s: got %dx%d display mode from %s\n", __func__, vm.hactive, > + vm.vactive, np->name); > + dump_drm_displaymode(dmode); > + > + return 0; > + > +} > +EXPORT_SYMBOL_GPL(of_get_drm_display_mode); > +#else > +int videomode_to_display_mode(struct videomode *vm, struct drm_display_mode > *dmode) > +{ > + return 0; > +} > + > +int of_get_drm_display_mode(struct device_node *np, struct drm_display_mode > *dmode, > + int index) > +{ > + return 0; > +} > +#endif > + > +int videomode_to_fb_videomode(struct videomode *vm, struct fb_videomode > *fbmode) > +{ > + memset(fbmode, 0, sizeof(*fbmode)); > + > + fbmode->xres = vm->hactive; > + fbmode->left_margin = vm->hback_porch; > + fbmode->right_margin = vm->hfront_porch; > + fbmode->hsync_len = vm->hsync_len; > + > + fbmode->yres = vm->vactive; > + fbmode->upper_margin = vm->vback_porch; > + fbmode->lower_margin = vm->vfront_porch; > + fbmode->vsync_len = vm->vsync_len; > + > + fbmode->pixclock = KHZ2PICOS(vm->pixelclock) / 1000; > + > + if (vm->hah) > + fbmode->sync |= FB_SYNC_HOR_HIGH_ACT; > + if (vm->vah) > + fbmode->sync |= FB_SYNC_VERT_HIGH_ACT; > + if (vm->interlaced) > + fbmode->vmode |= FB_VMODE_INTERLACED; > + if (vm->doublescan) > + fbmode->vmode |= FB_VMODE_DOUBLE; > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(videomode_to_fb_videomode); > + > +int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb, > + int index) > +{ > + struct videomode vm; > + int ret; > + > + ret = of_get_videomode(np, , index); > + if (ret) > + return ret; > + > + videomode_to_fb_videomode(, fb); > + > + pr_info("%s: got %dx%d display mode from %s\n", __func__, vm.hactive, > + vm.vactive, np->name); > + dump_fb_videomode(fb); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(of_get_drm_display_mode); > diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h > new file mode 100644 > index 000..96efe01 > --- /dev/null > +++ b/include/linux/of_videomode.h > @@ -0,0 +1,41 @@ > +/* > + * Copyright 2012 Steffen Trumtrar > + * > + * generic videomode description > + * > + * This file is released under the GPLv2 > + */ > + > +#ifndef __LINUX_VIDEOMODE_H > +#define __LINUX_VIDEOMODE_H > + > +#include You don't need to include this. > +struct videomode { > + u32 pixelclock; > + u32 refreshrate; > + > + u32 hactive; > + u32 hfront_porch; > + u32 hback_porch; > + u32 hsync_len; > + > + u32 vactive; > + u32 vfront_porch; > + u32 vback_porch; > + u32 vsync_len; > + > + bool hah; > + bool vah; > + bool interlaced; > + bool doublescan; > + > +}; This is not really of related. And actually, neither is the struct signal_timing in the previous patch. It would be nice to have these in a common header that fb, drm, and others could use instead of each having their own timing structs. But that's probably out of scope for this series =). Did you check the timing structs from the video related frameworks in the kernel to see if your structs contain all the info the others have, so that, at least in theory, everybody could use these common structs? Tomi -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121008/90b4a17f/attachment.pgp>
[PATCH 1/2 v6] of: add helper to parse display timings
On Mon, 2012-10-08 at 10:07 +0300, Tomi Valkeinen wrote: > Hi, > I don't see you setting disp->default_timing to OF_DEFAULT_TIMING in > case there's no default_timing found. > > Or, at least I presume OF_DEFAULT_TIMING is meant to mark non-existing > default timing. The name OF_DEFAULT_TIMING is not very descriptive to > me. Ah, I see now from the second patch how this is meant to be used. So if there's no default timing in DT data, disp->default_timing is 0, meaning the first entry. And the caller of of_get_videomode() will use OF_DEFAULT_TIMING as index to get the default mode. So I think it's ok. Tomi -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121008/1b37c16b/attachment.pgp>
[PATCH 1/2 v6] of: add helper to parse display timings
On 10/08/2012 06:20 AM, Tomi Valkeinen wrote: > On Mon, 2012-10-08 at 14:04 +0200, Laurent Pinchart wrote: >> On Monday 08 October 2012 12:01:18 Tomi Valkeinen wrote: >>> On Mon, 2012-10-08 at 10:25 +0200, Guennadi Liakhovetski >>> wrote: ... >>> Of course, if this is about describing the hardware, the >>> default-mode property doesn't really fit in... >> >> Maybe we should rename it to native-mode then ? > > Hmm, right, if it means native mode, then it is describing the > hardware. But would it make sense to require that the native mode > is the first mode in the list, then? This would make the separate > default-mode/native-mode property not needed. I'm not sure if device-tree guarantees that the nodes enumerate in a specific order. If it does, then that may be a reasonable solution.
[PATCH 1/2 v6] of: add helper to parse display timings
On 10/08/2012 02:25 AM, Guennadi Liakhovetski wrote: > On Fri, 5 Oct 2012, Stephen Warren wrote: > >> On 10/04/2012 03:35 PM, Guennadi Liakhovetski wrote: >>> Hi Steffen >>> >>> Sorry for chiming in so late in the game, but I've long been wanting to >>> have a look at this and compare with what we do for V4L2, so, this seems a >>> great opportunity to me:-) >>> >>> On Thu, 4 Oct 2012, Steffen Trumtrar wrote: >> diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt >> +timings-subnode +--- + +required properties: + - hactive, vactive: Display resolution + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters + in pixels + vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in + lines + - clock: displayclock in Hz >>> >>> You're going to hate me for this, but eventually we want to actually >>> reference clock objects in our DT bindings. For now, even if you don't >>> want to actually add clock phandles and stuff here, I think, using the >>> standard "clock-frequency" property would be much better! >> >> In a definition of a display timing, we will never need to use the clock >> binding; the clock binding would be used by the HW module that is >> generating a timing, not by the timing definition itself. > > You mean clock consumer bindings will be in the display device DT node? > And the display-timings node will be its child? Yes ... + - interlaced (bool) >>> >>> Is "interlaced" a property of the hardware, i.e. of the board? Can the >>> same display controller on one board require interlaced data and on >>> another board - progressive? >> >> Interlace is a property of a display mode. It's quite possible for a >> particular display controller to switch between interlace and >> progressive output at run-time. For example, reconfiguring the output >> between 480i, 720p, 1080i, 1080p modes. Admittedly, if you're talking to >> a built-in LCD display, you're probably always going to be driving the >> single mode required by the panel, and that mode will likely always be >> progressive. However, since this binding attempts to describe any >> display timing, I think we still need this property per mode. > > But why do you need this in the DT then at all? Because the driver for the display controller has no idea what display or panel will be connected to it. > If it's fixed, as required > per display controller, then its driver will know it. If it's runtime > configurable, then it's a purely software parameter and doesn't depend on > the board? interlace-vs-progressive isn't "fixed, as required per display controller", but is a property of the mode being sent by the display controller, and the requirements for that mode are driven by the panel/display connected to the display controller, not the display controller, in general. ... >>> BTW, I'm not very familiar with display >>> interfaces, but for interlaced you probably sometimes use a field signal, >>> whose polarity you also want to specify here? We use a "field-even-active" >>> integer property for it. >> >> I think that's a property of the display controller itself, rather than >> an individual mode, although I'm not 100% certain. My assertion is that >> the physical interface that the display controller is driving will >> determine whether embedded or separate sync is used, and in the separate >> sync case, how the field signal is defined, and that all interlace modes >> driven over that interface will use the same field signal definition. > > In general, I might be misunderstanding something, but don't we have to > distinguish between 2 types of information about display timings: (1) is > defined by the display controller requirements, is known to the display > driver and doesn't need to be present in timings DT. We did have some of > these parameters in board data previously, because we didn't have proper > display controller drivers... Yes, there probably is data of that kind, but the display mode timings binding is only address standardized display timings information, not controller-specific information, and hence doesn't cover this case. > (2) is board specific configuration, and is > such it has to be present in DT. Certainly, yes. > In that way, doesn't "interlaced" belong to type (1) and thus doesn't need > to be present in DT? I don't believe so.
[PATCH 1/2 v6] of: add helper to parse display timings
> + pr_err("%s: could not find display-timings node\n", __func__); > + return NULL; > + } > + > + disp = kzalloc(sizeof(*disp), GFP_KERNEL); > + > + entry = of_parse_phandle(timings_np, "default-timing", 0); > + > + if (!entry) { > + pr_info("%s: no default-timing specified\n", __func__); > + entry = of_find_node_by_name(np, "timing"); > + } If "default-timing" property is optional, I don't see any need for the pr_info above, as it should be business as usual if the property doesn't exist. If the default-timing property doesn't exist, wouldn't it be simpler to get the first subnode, instead of looking one with "timing" name? > + > + if (!entry) { > + pr_info("%s: no timing specifications given\n", __func__); > + return disp; > + } Again, I don't think the pr_info is needed if this is a normal case. Then again, perhaps this could be an error? Why would there be a display node without any timings? > + > + pr_info("%s: using %s as default timing\n", __func__, entry->name); > + > + default_timing = (char *)entry->full_name; const char *? > + > + disp->num_timings = 0; > + > + for_each_child_of_node(timings_np, entry) { > + disp->num_timings++; > + } No need for { } > + disp->timings = kzalloc(sizeof(struct signal_timing > *)*disp->num_timings, > + GFP_KERNEL); > + > + disp->num_timings = 0; > + > + for_each_child_of_node(timings_np, entry) { > + struct signal_timing *st; > + > + st = of_get_display_timing(entry); > + > + if (!st) > + continue; > + > + if (strcmp(default_timing, entry->full_name) == 0) > + disp->default_timing = disp->num_timings; I don't see you setting disp->default_timing to OF_DEFAULT_TIMING in case there's no default_timing found. Or, at least I presume OF_DEFAULT_TIMING is meant to mark non-existing default timing. The name OF_DEFAULT_TIMING is not very descriptive to me. Would it make more sense to have the disp->default_timing as a pointer to the timing, instead of index? Then a NULL value would mark a non-existing default timing. > + disp->timings[disp->num_timings] = st; > + disp->num_timings++; > + } > + > + > + of_node_put(timings_np); > + > + if (disp->num_timings >= 0) > + pr_info("%s: got %d timings. Using timing #%d as default\n", > __func__, > + disp->num_timings , disp->default_timing + 1); > + else > + pr_info("%s: no timings specified\n", __func__); > + > + return disp; > +} > +EXPORT_SYMBOL_GPL(of_get_display_timing_list); > + > +int of_display_timings_exists(struct device_node *np) > +{ > + struct device_node *timings_np; > + struct device_node *default_np; > + > + if (!np) > + return -EINVAL; > + > + timings_np = of_parse_phandle(np, "display-timings", 0); > + > + if (!timings_np) > + return -EINVAL; > + > + default_np = of_parse_phandle(np, "default-timing", 0); > + > + if (default_np) > + return 0; > + > + return -EINVAL; > +} > +EXPORT_SYMBOL_GPL(of_display_timings_exists); > diff --git a/include/linux/of_display_timings.h > b/include/linux/of_display_timings.h > new file mode 100644 > index 000..1ad719e > --- /dev/null > +++ b/include/linux/of_display_timings.h > @@ -0,0 +1,85 @@ > +/* > + * Copyright 2012 Steffen Trumtrar > + * > + * description of display timings > + * > + * This file is released under the GPLv2 > + */ > + > +#ifndef __LINUX_OF_DISPLAY_TIMINGS_H > +#define __LINUX_OF_DISPLAY_TIMINGS_H > + > +#define OF_DEFAULT_TIMING -1 > + > +struct display_timings { > + unsigned int num_timings; > + unsigned int default_timing; > + > + struct signal_timing **timings; Should this be a pointer to a const array of const data? Is there ever need to change them after they've been read from DT? > +}; > + > +struct timing_entry { > + u32 min; > + u32 typ; > + u32 max; > +}; > + > +struct signal_timing { > + struct timing_entry pixelclock; > + > + struct timing_entry hactive; > + struct timing_entry hfront_porch; > + struct timing_entry hback_porch; > + struct timing_entry hsync_len; > + > + struct timing_entry vactive; > + struct timing_entry vfront_porch; > + struct timing_entry vback_porch; > + struct timing_entry vsync_len; > + > + bool vsync_pol_active_high; > + bool hsync_pol_active_high; > + bool de_pol_active_high; > + bool pixelclk_pol_inverted; > + bool interlaced; > + bool doublescan; > +}; > + > +struct display_timings *of_get_display_timing_list(struct device_node *np); > +struct signal_timing *of_get_display_timing(struct device_node *np); > +int of_display_timings_exists(struct device_node *np); > + > +/* placeholder function until ranges are really needed */ > +static inline u32 signal_timing_get_value(struct timing_entry *te, int index) > +{ > + return te->typ; > +} > + > +static inline void timings_release(struct display_timings *disp) > +{ > + int i; > + > + for (i = 0; i < disp->num_timings; i++) > + kfree(disp->timings[i]); > +} > + > +static inline void display_timings_release(struct display_timings *disp) > +{ > + timings_release(disp); > + kfree(disp->timings); > +} > + > +static inline struct signal_timing *display_timings_get(struct > display_timings *disp, > + int index) > +{ > + struct signal_timing *st; > + > + if (disp->num_timings > index) { > + st = disp->timings[index]; > + return st; > + } > + else > + return NULL; > +} Why do you have these functions in the header file? Tomi -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121008/586caabb/attachment-0001.pgp>
[PATCH 2/2 v6] of: add generic videomode description
On Mon, Oct 08, 2012 at 10:21:53AM +0300, Tomi Valkeinen wrote: > On Thu, 2012-10-04 at 19:59 +0200, Steffen Trumtrar wrote: > > Get videomode from devicetree in a format appropriate for the > > backend. drm_display_mode and fb_videomode are supported atm. > > Uses the display signal timings from of_display_timings > > > > Signed-off-by: Steffen Trumtrar > > --- > > drivers/of/Kconfig |5 + > > drivers/of/Makefile |1 + > > drivers/of/of_videomode.c| 212 > > ++ > > include/linux/of_videomode.h | 41 > > 4 files changed, 259 insertions(+) > > create mode 100644 drivers/of/of_videomode.c > > create mode 100644 include/linux/of_videomode.h > > > > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig > > index 646deb0..74282e2 100644 > > --- a/drivers/of/Kconfig > > +++ b/drivers/of/Kconfig > > @@ -88,4 +88,9 @@ config OF_DISPLAY_TIMINGS > > help > > helper to parse display timings from the devicetree > > > > +config OF_VIDEOMODE > > + def_bool y > > + help > > + helper to get videomodes from the devicetree > > + > > endmenu # OF > > diff --git a/drivers/of/Makefile b/drivers/of/Makefile > > index c8e9603..09d556f 100644 > > --- a/drivers/of/Makefile > > +++ b/drivers/of/Makefile > > @@ -12,3 +12,4 @@ obj-$(CONFIG_OF_PCI) += of_pci.o > > obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o > > obj-$(CONFIG_OF_MTD) += of_mtd.o > > obj-$(CONFIG_OF_DISPLAY_TIMINGS) += of_display_timings.o > > +obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o > > diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c > > new file mode 100644 > > index 000..76ac16e > > --- /dev/null > > +++ b/drivers/of/of_videomode.c > > @@ -0,0 +1,212 @@ > > +/* > > + * generic videomode helper > > + * > > + * Copyright (c) 2012 Steffen Trumtrar , > > Pengutronix > > + * > > + * This file is released under the GPLv2 > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +void dump_fb_videomode(struct fb_videomode *m) > > +{ > > +pr_debug("fb_videomode = %d %d %d %d %d %d %d %d %d %d %d %d %d\n", > > +m->refresh, m->xres, m->yres, m->pixclock, m->left_margin, > > +m->right_margin, m->upper_margin, m->lower_margin, > > +m->hsync_len, m->vsync_len, m->sync, m->vmode, m->flag); > > +} > > + > > +void dump_drm_displaymode(struct drm_display_mode *m) > > +{ > > +pr_debug("drm_displaymode = %d %d %d %d %d %d %d %d %d\n", > > +m->hdisplay, m->hsync_start, m->hsync_end, m->htotal, > > +m->vdisplay, m->vsync_start, m->vsync_end, m->vtotal, > > +m->clock); > > +} > > + > > +int videomode_from_timing(struct display_timings *disp, struct videomode > > *vm, > > + int index) > > +{ > > + struct signal_timing *st = NULL; > > + > > + if (!vm) > > + return -EINVAL; > > + > > + st = display_timings_get(disp, index); > > + > > + if (!st) { > > + pr_err("%s: no signal timings found\n", __func__); > > + return -EINVAL; > > + } > > + > > + vm->pixelclock = signal_timing_get_value(>pixelclock, 0); > > + vm->hactive = signal_timing_get_value(>hactive, 0); > > + vm->hfront_porch = signal_timing_get_value(>hfront_porch, 0); > > + vm->hback_porch = signal_timing_get_value(>hback_porch, 0); > > + vm->hsync_len = signal_timing_get_value(>hsync_len, 0); > > + > > + vm->vactive = signal_timing_get_value(>vactive, 0); > > + vm->vfront_porch = signal_timing_get_value(>vfront_porch, 0); > > + vm->vback_porch = signal_timing_get_value(>vback_porch, 0); > > + vm->vsync_len = signal_timing_get_value(>vsync_len, 0); > > + > > + vm->vah = st->vsync_pol_active_high; > > + vm->hah = st->hsync_pol_active_high; > > + vm->interlaced = st->interlaced; > > + vm->doublescan = st->doublescan; > > + > > + return 0; > > +} > > + > > +int of_get_videomode(struct device_node *np, struct videomode *vm, int > > index) > > +{ > > + struct display_timings *disp; > > + int ret = 0; > > + > > + disp = of_get_display_timing_list(np); > > + > > + if (!disp) { > > + pr_err("%s: no timings specified\n", __func__); > > + return -EINVAL; > > + } > > + > > + if (index == OF_DEFAULT_TIMING) > > + index = disp->default_timing; > > + > > + ret = videomode_from_timing(disp, vm, index); > > + > > + if (ret) > > + return ret; > > + > > + display_timings_release(disp); > > + > > + if (!vm) { > > + pr_err("%s: could not get videomode %d\n", __func__, index); > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(of_get_videomode); > > + > > +#if defined(CONFIG_DRM) > > +int videomode_to_display_mode(struct videomode *vm, struct > > drm_display_mode *dmode) > > +{ > > + memset(dmode, 0, sizeof(*dmode)); > > + > > + dmode->hdisplay
[Bug 50149] Faulty shaders on RS600
https://bugs.freedesktop.org/show_bug.cgi?id=50149 --- Comment #24 from Roman ?makal --- Torchlight seems to have same rendering issues as seen on HoN video. Time after time textures and models flickers. Playable though .. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121008/ddadc61b/attachment.html>
[PATCH] drm/radeon: update comments to clarify VM setup
From: Alex DeucherThe actual set up and assignment of VM page tables is done on the fly in radeon_gart.c. Signed-off-by: Alex Deucher --- drivers/gpu/drm/radeon/ni.c|4 drivers/gpu/drm/radeon/radeon_device.c |3 +++ drivers/gpu/drm/radeon/si.c|7 --- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c index 48e2337..cfb9276 100644 --- a/drivers/gpu/drm/radeon/ni.c +++ b/drivers/gpu/drm/radeon/ni.c @@ -770,6 +770,10 @@ static int cayman_pcie_gart_enable(struct radeon_device *rdev) WREG32(0x15DC, 0); /* empty context1-7 */ + /* Assign the pt base to something valid for now; the pts used for +* the VMs are determined by the application and setup and assigned +* on the fly in the vm part of radeon_gart.c +*/ for (i = 1; i < 8; i++) { WREG32(VM_CONTEXT0_PAGE_TABLE_START_ADDR + (i << 2), 0); WREG32(VM_CONTEXT0_PAGE_TABLE_END_ADDR + (i << 2), 0); diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 64a4264..1fad47e 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -1018,6 +1018,9 @@ int radeon_device_init(struct radeon_device *rdev, return r; /* initialize vm here */ mutex_init(>vm_manager.lock); + /* FIXME start with 4G, once using 2 level pt switch to full +* vm size space +*/ rdev->vm_manager.max_pfn = 1 << 20; INIT_LIST_HEAD(>vm_manager.lru_vm); diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c index c76825f..f272ead 100644 --- a/drivers/gpu/drm/radeon/si.c +++ b/drivers/gpu/drm/radeon/si.c @@ -2407,12 +2407,13 @@ static int si_pcie_gart_enable(struct radeon_device *rdev) WREG32(0x15DC, 0); /* empty context1-15 */ - /* FIXME start with 4G, once using 2 level pt switch to full -* vm size space -*/ /* set vm size, must be a multiple of 4 */ WREG32(VM_CONTEXT1_PAGE_TABLE_START_ADDR, 0); WREG32(VM_CONTEXT1_PAGE_TABLE_END_ADDR, rdev->vm_manager.max_pfn); + /* Assign the pt base to something valid for now; the pts used for +* the VMs are determined by the application and setup and assigned +* on the fly in the vm part of radeon_gart.c +*/ for (i = 1; i < 16; i++) { if (i < 8) WREG32(VM_CONTEXT0_PAGE_TABLE_BASE_ADDR + (i << 2), -- 1.7.7.5
[PATCH 1/2 v6] of: add helper to parse display timings
On Sun, Oct 07, 2012 at 03:38:25PM +0200, Laurent Pinchart wrote: > Hi Steffen, > > On Friday 05 October 2012 18:38:58 Steffen Trumtrar wrote: > > On Fri, Oct 05, 2012 at 10:21:37AM -0600, Stephen Warren wrote: > > > On 10/05/2012 10:16 AM, Steffen Trumtrar wrote: > > > > On Thu, Oct 04, 2012 at 12:47:16PM -0600, Stephen Warren wrote: > > > >> On 10/04/2012 11:59 AM, Steffen Trumtrar wrote: > > > ... > > > > > > >>> + for_each_child_of_node(timings_np, entry) { > > > >>> + struct signal_timing *st; > > > >>> + > > > >>> + st = of_get_display_timing(entry); > > > >>> + > > > >>> + if (!st) > > > >>> + continue; > > > >> > > > >> I wonder if that shouldn't be an error? > > > > > > > > In the sense of a pr_err not a -EINVAL I presume?! It is a little bit > > > > quiet in case of a faulty spec, that is right. > > > > > > I did mean return an error; if we try to parse something and can't, > > > shouldn't we return an error? > > > > > > I suppose it may be possible to limp on and use whatever subset of modes > > > could be parsed and drop the others, which is what this code does, but > > > the code after the loop would definitely return an error if zero timings > > > were parseable. > > > > If a display supports multiple modes, I think it is better to have a working > > mode (even if it is not the preferred one) than have none at all. > > If there is no mode at all, that should be an error, right. > > If we fail completely in case of an error, DT writers will notice their bugs. > If we ignore errors silently they won't, and we'll end up with buggy DTs (or, > to be accurate, even more buggy DTs :-)). I'd rather fail completely in the > first implementation and add workarounds later only if we need to. > Okay, that is two against one. And if you say it like this, Stephen and you are right I guess. Fail completely it is then. Regards, Steffen -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[PATCH] drm/radeon: allocate page tables on demand v3
On Mon, Oct 8, 2012 at 5:55 AM, Christian K?nig wrote: > Got time today to test that a bit more, no piglit regressions on SI so far. > > And also CAYMAN still seems to work with it, but there are allot of piglit > tests there that still doesn't work when enabling VM (and it doesn't matter > if you have this patch or not). > > Any objections to pull that into 3.7 as well? Looks good to me. I'll give it some more testing today. Alex
[PATCH 3/3] Android.mk: use LOCAL_COPY_HEADERS to export headers.
From: Haitao HuangExport necessary header files used by other components for Android, such as libva intel-driver, gralloc, hwcomposer, etc. Change-Id: I2feabf6941379ef4d756e942f30eba059de641f1 Signed-off-by: Haitao Huang [chad: Fixed inconsistent indentation.] Signed-off-by: Chad Versace --- Android.mk | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Android.mk b/Android.mk index 1029dc6..68db5df 100644 --- a/Android.mk +++ b/Android.mk @@ -1,5 +1,5 @@ # -# Copyright ? 2011 Intel Corporation +# Copyright ? 2011-2012 Intel Corporation # # Permission is hereby granted, free of charge, to any person obtaining a # copy of this software and associated documentation files (the "Software"), @@ -45,6 +45,16 @@ LOCAL_C_INCLUDES := \ LOCAL_CFLAGS := \ -DHAVE_LIBDRM_ATOMIC_PRIMITIVES=1 +LOCAL_COPY_HEADERS :=\ + xf86drm.h\ + include/drm/drm_fourcc.h \ + include/drm/drm.h\ + include/drm/drm_mode.h \ + include/drm/drm_sarea.h \ + include/drm/i915_drm.h \ + intel/intel_bufmgr.h \ + +LOCAL_COPY_HEADERS_TO := libdrm include $(BUILD_SHARED_LIBRARY) include $(LOCAL_PATH)/intel/Android.mk -- 1.7.11.7
[PATCH 2/3] libdrm,intel: Add Android makefiles (v2)
From: Chad VersaceThis enables libdrm.so and libdrm_intel.so to build on Android IceCreamSandwich. v2: Link libdrm_intel to libpciaccess. Signed-off-by: Chad Versace --- Android.mk | 52 +++ intel/Android.mk | 57 2 files changed, 109 insertions(+) create mode 100644 Android.mk create mode 100644 intel/Android.mk diff --git a/Android.mk b/Android.mk new file mode 100644 index 000..1029dc6 --- /dev/null +++ b/Android.mk @@ -0,0 +1,52 @@ +# +# Copyright ? 2011 Intel Corporation +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice (including the next +# paragraph) shall be included in all copies or substantial portions of the +# Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +# IN THE SOFTWARE. +# + +LIBDRM_VALID_GPU_DRIVERS := i915 i965 + +# Skip this makefile if we're not building for a known GPU. +ifneq ($(filter $(BOARD_GPU_DRIVERS), $(LIBDRM_VALID_GPU_DRIVERS)),) + +LOCAL_PATH := $(call my-dir) +include $(CLEAR_VARS) + +LIBDRM_TOP := $(LOCAL_PATH) + +# Import variable LIBDRM_SOURCES. +include $(LOCAL_PATH)/sources.mk + +LOCAL_MODULE := libdrm +LOCAL_MODULE_TAGS := optional + +LOCAL_SRC_FILES := $(LIBDRM_SOURCES) + +LOCAL_C_INCLUDES := \ + $(LIBDRM_TOP)/include/drm + +LOCAL_CFLAGS := \ + -DHAVE_LIBDRM_ATOMIC_PRIMITIVES=1 + +include $(BUILD_SHARED_LIBRARY) + +include $(LOCAL_PATH)/intel/Android.mk + +endif # BOARD_GPU_DRIVERS diff --git a/intel/Android.mk b/intel/Android.mk new file mode 100644 index 000..3647a14 --- /dev/null +++ b/intel/Android.mk @@ -0,0 +1,57 @@ +# +# Copyright ? 2011 Intel Corporation +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice (including the next +# paragraph) shall be included in all copies or substantial portions of the +# Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +# IN THE SOFTWARE. +# + +LIBDRM_INTEL_VALID_GPU_DRIVERS := i915 i965 + +# Skip this makefile if we're not building for an Intel GPU. +ifneq ($(filter $(BOARD_GPU_DRIVERS), $(LIBDRM_INTEL_VALID_GPU_DRIVERS)),) + +LOCAL_PATH := $(call my-dir) +include $(CLEAR_VARS) + +# Import variable LIBDRM_INTEL_SOURCES. +include $(LOCAL_PATH)/sources.mk + +LOCAL_MODULE := libdrm_intel +LOCAL_MODULE_TAGS := optional + +LOCAL_SHARED_LIBRARIES := libdrm + +LOCAL_SRC_FILES := $(LIBDRM_INTEL_SOURCES) + +LOCAL_C_INCLUDES := \ + $(LIBDRM_TOP) \ + $(LIBDRM_TOP)/intel \ + $(LIBDRM_TOP)/include/drm \ + external/libpciaccess/include + +LOCAL_CFLAGS := \ + -DHAVE_LIBDRM_ATOMIC_PRIMITIVES=1 + +LOCAL_SHARED_LIBRARIES := \ + libdrm \ + libpciaccess + +include $(BUILD_SHARED_LIBRARY) + +endif # BOARD_GPU_DRIVERS -- 1.7.11.7
[PATCH 1/3] libdrm,intel: Factor source file lists into sources.mk
From: Chad VersaceFactor the source file list for libdrm.so from Makefile.am into sources.mk. Ditto for libdrm_intel.so. This is in preparation for adding support for Android. The sources.mk's will be shared between autotools and Android. Rationale: The most commonly changed parts of any makefile are the source lists. So, by sharing the lists between the two build systems, we can reduce the frequency at which modifications to the Linux build breaks the Android build. Signed-off-by: Chad Versace Signed-off-by: Sean V Kelley Signed-off-by: Tapani P?lli --- Makefile.am | 9 - intel/Makefile.am | 9 - intel/sources.mk | 30 ++ sources.mk| 30 ++ 4 files changed, 68 insertions(+), 10 deletions(-) create mode 100644 intel/sources.mk create mode 100644 sources.mk diff --git a/Makefile.am b/Makefile.am index 8ecd9d9..b854703 100644 --- a/Makefile.am +++ b/Makefile.am @@ -49,6 +49,9 @@ if HAVE_EXYNOS EXYNOS_SUBDIR = exynos endif +# Import variable LIBDRM_SOURCES. +include sources.mk + SUBDIRS = . $(LIBKMS_SUBDIR) $(INTEL_SUBDIR) $(NOUVEAU_SUBDIR) $(RADEON_SUBDIR) $(OMAP_SUBDIR) $(EXYNOS_SUBDIR) tests include man libdrm_la_LTLIBRARIES = libdrm.la @@ -59,11 +62,7 @@ libdrm_la_LIBADD = @CLOCK_LIB@ libdrm_la_CPPFLAGS = -I$(top_srcdir)/include/drm libdrm_la_SOURCES =\ - xf86drm.c \ - xf86drmHash.c \ - xf86drmRandom.c \ - xf86drmSL.c \ - xf86drmMode.c \ + $(LIBDRM_SOURCES) \ xf86atomic.h\ libdrm_lists.h diff --git a/intel/Makefile.am b/intel/Makefile.am index f49b099..e937c4b 100644 --- a/intel/Makefile.am +++ b/intel/Makefile.am @@ -22,6 +22,9 @@ # Authors: #Eric Anholt +# Import variable LIBDRM_INTEL_SOURCES. +include sources.mk + AM_CFLAGS = \ $(WARN_CFLAGS) \ -I$(top_srcdir) \ @@ -40,13 +43,9 @@ libdrm_intel_la_LIBADD = ../libdrm.la \ @CLOCK_LIB@ libdrm_intel_la_SOURCES = \ - intel_bufmgr.c \ + $(LIBDRM_INTEL_SOURCES) \ intel_bufmgr_priv.h \ - intel_bufmgr_fake.c \ - intel_bufmgr_gem.c \ - intel_decode.c \ intel_chipset.h \ - mm.c \ mm.h intel_bufmgr_gem_o_CFLAGS = $(AM_CFLAGS) -c99 diff --git a/intel/sources.mk b/intel/sources.mk new file mode 100644 index 000..2f6f744 --- /dev/null +++ b/intel/sources.mk @@ -0,0 +1,30 @@ +# +# Copyright ? 2012 Intel Corporation +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice (including the next +# paragraph) shall be included in all copies or substantial portions of the +# Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +# IN THE SOFTWARE. +# + +# for libdrm_intel.so +LIBDRM_INTEL_SOURCES := \ + intel_bufmgr.c \ + intel_bufmgr_fake.c \ + intel_bufmgr_gem.c \ + intel_decode.c \ + mm.c diff --git a/sources.mk b/sources.mk new file mode 100644 index 000..19aa059 --- /dev/null +++ b/sources.mk @@ -0,0 +1,30 @@ +# +# Copyright ? 2012 Intel Corporation +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice (including the next +# paragraph) shall be included in all copies or substantial portions of the +# Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
[PATCH 0/3] Android support
Upstreaming old set of patches here to enable Android support in libdrm. Some little rebasing was required for the first one. Chad Versace (2): libdrm,intel: Factor source file lists into sources.mk libdrm,intel: Add Android makefiles (v2) Haitao Huang (1): Android.mk: use LOCAL_COPY_HEADERS to export headers. Android.mk| 62 +++ Makefile.am | 9 intel/Android.mk | 57 ++ intel/Makefile.am | 9 intel/sources.mk | 30 +++ sources.mk| 30 +++ 6 files changed, 187 insertions(+), 10 deletions(-) create mode 100644 Android.mk create mode 100644 intel/Android.mk create mode 100644 intel/sources.mk create mode 100644 sources.mk -- 1.7.11.7
[PATCH 0/3] Android support
Tapani P?lli writes: > Upstreaming old set of patches here to enable Android support in libdrm. > Some little rebasing was required for the first one. > > Chad Versace (2): > libdrm,intel: Factor source file lists into sources.mk > libdrm,intel: Add Android makefiles (v2) Couldn't this just be done with androgenizer without making a maintenance mess in the upstream code? -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121008/aacac3d6/attachment.pgp>
[PATCH 1/2 v6] of: add helper to parse display timings
On 10/7/2012 11:01 PM, Tomi Valkeinen wrote: > On Mon, 2012-10-08 at 10:25 +0200, Guennadi Liakhovetski wrote: > >> In general, I might be misunderstanding something, but don't we have to >> distinguish between 2 types of information about display timings: (1) is >> defined by the display controller requirements, is known to the display >> driver and doesn't need to be present in timings DT. We did have some of >> these parameters in board data previously, because we didn't have proper >> display controller drivers... (2) is board specific configuration, and is >> such it has to be present in DT. >> >> In that way, doesn't "interlaced" belong to type (1) and thus doesn't need >> to be present in DT? > > As I see it, this DT data is about the display (most commonly LCD > panel), i.e. what video mode(s) the panel supports. If things were done > my way, the panel's supported timings would be defined in the driver for > the panel, and DT would be left to describe board specific data, but > this approach has its benefits. > > Thus, if you connect an interlaced panel to your board, Do interlaced panels exist? I have never seen one. you need to tell > the display controller that this panel requires interlace signal. Also, > pixel clock source doesn't make sense in this context, as this doesn't > describe the actual used configuration, but only what the panel > supports. > > Of course, if this is about describing the hardware, the default-mode > property doesn't really fit in... > > Tomi > > > > ___ > devicetree-discuss mailing list > devicetree-discuss at lists.ozlabs.org > https://lists.ozlabs.org/listinfo/devicetree-discuss >
[git pull] drm for 3.7-rc1 (part 2)
Hi Linus, This is the follow-up pull, 3 pieces a) exynos next stuff, was delayed but looks okay to me, one patch in v4l bits but it was acked by v4l person. b) UAPI disintegration bits c) intel fixes - DP fixes, hang fixes, other misc fixes. Dave. The following changes since commit 0b8e74c6f44094189dbe78baf4101acc7570c6af: Merge branch 'v4l_for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media (2012-10-07 17:49:05 +0900) are available in the git repository at: git://people.freedesktop.org/~airlied/linux.git drm-next for you to fetch changes up to 1f31c69dac71bebc0f00bc8534a6345782045501: Merge branch 'drm-intel-fixes' of git://people.freedesktop.org/~danvet/drm-intel into drm-next (2012-10-07 21:13:54 +1000) Adam Jackson (6): drm: Export drm_probe_ddc() drm/dp: Update DPCD defines drm/i915/dp: Fetch downstream port info if needed during DPCD fetch drm/i915/dp: Be smarter about connection sense for branch devices drm/dp: Document DP spec versions for various DPCD registers drm/dp: Make sink count DP 1.2 aware Ben Widawsky (2): drm/i915: Fix set_caching locking drm/i915: Fix GT_MODE default value Chris Wilson (3): drm/i915: Actually invalidate the TLB for the SandyBridge HW contexts w/a drm/i915: Flush the pending flips on the CRTC before modification drm/i915: Try harder to complete DP training pattern 1 Daniel Vetter (2): drm/i915: call drm_handle_vblank before finish_page_flip drm/i915: don't frob the vblank ts in finish_page_flip Dave Airlie (3): Merge branch 'disintegrate-drm' of git://git.infradead.org/users/dhowells/linux-headers into drm-next Merge branch 'exynos-drm-next' of git://git.infradead.org/users/kmpark/linux-samsung into drm-next Merge branch 'drm-intel-fixes' of git://people.freedesktop.org/~danvet/drm-intel into drm-next David Howells (1): UAPI: (Scripted) Disintegrate include/drm Dmitry Rogozhkin (1): drm/i915: EBUSY status handling added to i915_gem_fault(). Inki Dae (16): drm/exynos: added device object to subdrv's remove callback as argument drm/exynos: separated subdrv_probe function into two parts. drm/exynos: separeated fimd_power_on into some parts. drm/exynos: fixed duplicated mode setting. drm/exynos: add wait_for_vblank callback interface. drm/exynos: make sure that hardware overlay for fimd is disabled drm/exynos: make sure that hardware overlay for hdmi is disabled drm/exynos: check NV12M format specific to Exynos properly drm/exynos: update crtc to plane safely drm/exynos: Disable plane when released drm/exynos: add pid to g2d_runqueue_node drm/exynos: fix duplicated mutex lock issue drm/exynos: check crtc's dpms mode at page flip drm/exynos: check crtc's dpms mode at SetCrtc drm/exynos: support drm_wait_vblank feature for VIDI drm/exynos: fix display power call issue. Jani Nikula (1): drm/i915: use adjusted_mode instead of mode for checking the 6bpc force flag Jesse Barnes (1): drm/i915: set swizzling to none on VLV Joonyoung Shim (2): drm/exynos: fix to calculate CRTC shown via screen drm/exynos: fix kcalloc size of g2d cmdlist node Leela Krishna Amudala (1): drm/exynos: add platform_device_id table and driver data for drm fimd Mika Kuoppala (1): drm/i915: print warning if vmi915_gem_fault error is not handled Rahul Sharma (9): drm: exynos: remove drm hdmi platform data struct drm: exynos: hdmi: add support for exynos5 ddc drm: exynos: hdmi: add support for exynos5 hdmiphy drm: exynos: hdmi: add support for platform variants for mixer drm: exynos: hdmi: add support to disable video processor in mixer drm: exynos: hdmi: add support for exynos5 mixer drm: exynos: hdmi: replace is_v13 with version check in hdmi drm: exynos: hdmi: add support for exynos5 hdmi drm: exynos: hdmi: remove drm common hdmi platform data struct Sachin Kamat (1): drm/exynos: Fix potential NULL pointer dereference Tomasz Stanislawski (5): media: s5p-hdmi: add HPD GPIO to platform data drm: exynos: hdmi: support for platform variants drm: exynos: hdmi: fix interrupt handling drm: exynos: hdmi: use s5p-hdmi platform data drm: exynos: hdmi: turn off HPD interrupt in HDMI chip drivers/gpu/drm/drm_edid.c|3 +- drivers/gpu/drm/exynos/exynos_ddc.c | 22 +- drivers/gpu/drm/exynos/exynos_drm_connector.c | 50 +- drivers/gpu/drm/exynos/exynos_drm_connector.h |4 + drivers/gpu/drm/exynos/exynos_drm_core.c | 100 ++- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 20 +- drivers/gpu/drm/exynos/exynos_drm_drv.h | 19 +- drivers/gpu/drm/exynos/exynos_drm_encoder.c | 116 ++-
Re: [PATCH 1/2 v6] of: add helper to parse display timings
Hi, On Thu, 2012-10-04 at 19:59 +0200, Steffen Trumtrar wrote: Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de --- .../devicetree/bindings/video/display-timings.txt | 222 drivers/of/Kconfig |5 + drivers/of/Makefile|1 + drivers/of/of_display_timings.c| 183 include/linux/of_display_timings.h | 85 5 files changed, 496 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt create mode 100644 drivers/of/of_display_timings.c create mode 100644 include/linux/of_display_timings.h diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt new file mode 100644 index 000..45e39bd --- /dev/null +++ b/Documentation/devicetree/bindings/video/display-timings.txt @@ -0,0 +1,222 @@ +display-timings bindings +== + +display-timings-node + + +required properties: + - none + +optional properties: + - default-timing: the default timing value + +timings-subnode +--- + +required properties: + - hactive, vactive: Display resolution + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters + in pixels + vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in + lines + - clock: displayclock in Hz + +optional properties: + - hsync-active-high (bool): Hsync pulse is active high + - vsync-active-high (bool): Vsync pulse is active high + - de-active-high (bool): Data-Enable pulse is active high + - pixelclk-inverted (bool): pixelclock is inverted + - interlaced (bool) + - doublescan (bool) I think bool should be generally used for things that are on/off, like interlace. For hsync-active-high others I'd rather have 0/1 values as others already suggested. +There are different ways of describing the capabilities of a display. The devicetree +representation corresponds to the one commonly found in datasheets for displays. +If a display supports multiple signal timings, the default-timing can be specified. + +The parameters are defined as + +struct signal_timing +=== + + +--+-+--+---+ + | |↑| | | + | ||vback_porch | | | + | |↓| | | + +--###--+---+ + | #↑# | | + | #|# | | + | hback #|# hfront | hsync | + | porch #| hactive # porch | len | + |#---+---#|-| + | #|# | | + | #|vactive # | | + | #|# | | + | #↓# | | + +--###--+---+ + | |↑| | | + | ||vfront_porch| | | + | |↓| | | + +--+-+--+---+ + | |↑| | | + | ||vsync_len | | | + | |↓| | | + +--+-+--+---+ + + +Example: + + display-timings { + default-timing = timing0; + timing0: 1920p24 { + /* 1920x1080p24 */ I think this is commonly called 1080p24. + clock = 5200; + hactive = 1920; + vactive = 1080; + hfront-porch = 25; + hback-porch = 25; + hsync-len = 25; + vback-porch = 2; + vfront-porch = 2; + vsync-len = 2; + hsync-active-high; + }; + }; + +Every property also supports the use of ranges, so the commonly used datasheet
Re: [PATCH 1/2 v6] of: add helper to parse display timings
On Mon, 2012-10-08 at 10:07 +0300, Tomi Valkeinen wrote: Hi, I don't see you setting disp-default_timing to OF_DEFAULT_TIMING in case there's no default_timing found. Or, at least I presume OF_DEFAULT_TIMING is meant to mark non-existing default timing. The name OF_DEFAULT_TIMING is not very descriptive to me. Ah, I see now from the second patch how this is meant to be used. So if there's no default timing in DT data, disp-default_timing is 0, meaning the first entry. And the caller of of_get_videomode() will use OF_DEFAULT_TIMING as index to get the default mode. So I think it's ok. Tomi signature.asc Description: This is a digitally signed message part ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2 v6] of: add generic videomode description
On Thu, 2012-10-04 at 19:59 +0200, Steffen Trumtrar wrote: Get videomode from devicetree in a format appropriate for the backend. drm_display_mode and fb_videomode are supported atm. Uses the display signal timings from of_display_timings Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de --- drivers/of/Kconfig |5 + drivers/of/Makefile |1 + drivers/of/of_videomode.c| 212 ++ include/linux/of_videomode.h | 41 4 files changed, 259 insertions(+) create mode 100644 drivers/of/of_videomode.c create mode 100644 include/linux/of_videomode.h diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index 646deb0..74282e2 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -88,4 +88,9 @@ config OF_DISPLAY_TIMINGS help helper to parse display timings from the devicetree +config OF_VIDEOMODE + def_bool y + help + helper to get videomodes from the devicetree + endmenu # OF diff --git a/drivers/of/Makefile b/drivers/of/Makefile index c8e9603..09d556f 100644 --- a/drivers/of/Makefile +++ b/drivers/of/Makefile @@ -12,3 +12,4 @@ obj-$(CONFIG_OF_PCI)+= of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o obj-$(CONFIG_OF_MTD) += of_mtd.o obj-$(CONFIG_OF_DISPLAY_TIMINGS) += of_display_timings.o +obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c new file mode 100644 index 000..76ac16e --- /dev/null +++ b/drivers/of/of_videomode.c @@ -0,0 +1,212 @@ +/* + * generic videomode helper + * + * Copyright (c) 2012 Steffen Trumtrar s.trumt...@pengutronix.de, Pengutronix + * + * This file is released under the GPLv2 + */ +#include linux/of.h +#include linux/fb.h +#include linux/slab.h +#include drm/drm_mode.h +#include linux/of_display_timings.h +#include linux/of_videomode.h + +void dump_fb_videomode(struct fb_videomode *m) +{ +pr_debug(fb_videomode = %d %d %d %d %d %d %d %d %d %d %d %d %d\n, +m-refresh, m-xres, m-yres, m-pixclock, m-left_margin, +m-right_margin, m-upper_margin, m-lower_margin, +m-hsync_len, m-vsync_len, m-sync, m-vmode, m-flag); +} + +void dump_drm_displaymode(struct drm_display_mode *m) +{ +pr_debug(drm_displaymode = %d %d %d %d %d %d %d %d %d\n, +m-hdisplay, m-hsync_start, m-hsync_end, m-htotal, +m-vdisplay, m-vsync_start, m-vsync_end, m-vtotal, +m-clock); +} + +int videomode_from_timing(struct display_timings *disp, struct videomode *vm, + int index) +{ + struct signal_timing *st = NULL; + + if (!vm) + return -EINVAL; + + st = display_timings_get(disp, index); + + if (!st) { + pr_err(%s: no signal timings found\n, __func__); + return -EINVAL; + } + + vm-pixelclock = signal_timing_get_value(st-pixelclock, 0); + vm-hactive = signal_timing_get_value(st-hactive, 0); + vm-hfront_porch = signal_timing_get_value(st-hfront_porch, 0); + vm-hback_porch = signal_timing_get_value(st-hback_porch, 0); + vm-hsync_len = signal_timing_get_value(st-hsync_len, 0); + + vm-vactive = signal_timing_get_value(st-vactive, 0); + vm-vfront_porch = signal_timing_get_value(st-vfront_porch, 0); + vm-vback_porch = signal_timing_get_value(st-vback_porch, 0); + vm-vsync_len = signal_timing_get_value(st-vsync_len, 0); + + vm-vah = st-vsync_pol_active_high; + vm-hah = st-hsync_pol_active_high; + vm-interlaced = st-interlaced; + vm-doublescan = st-doublescan; + + return 0; +} + +int of_get_videomode(struct device_node *np, struct videomode *vm, int index) +{ + struct display_timings *disp; + int ret = 0; + + disp = of_get_display_timing_list(np); + + if (!disp) { + pr_err(%s: no timings specified\n, __func__); + return -EINVAL; + } + + if (index == OF_DEFAULT_TIMING) + index = disp-default_timing; + + ret = videomode_from_timing(disp, vm, index); + + if (ret) + return ret; + + display_timings_release(disp); + + if (!vm) { + pr_err(%s: could not get videomode %d\n, __func__, index); + return -EINVAL; + } + + return 0; +} +EXPORT_SYMBOL_GPL(of_get_videomode); + +#if defined(CONFIG_DRM) +int videomode_to_display_mode(struct videomode *vm, struct drm_display_mode *dmode) +{ + memset(dmode, 0, sizeof(*dmode)); + + dmode-hdisplay = vm-hactive; + dmode-hsync_start = dmode-hdisplay + vm-hfront_porch; + dmode-hsync_end = dmode-hsync_start + vm-hsync_len; + dmode-htotal = dmode-hsync_end + vm-hback_porch; + + dmode-vdisplay = vm-vactive; + dmode-vsync_start = dmode-vdisplay + vm-vfront_porch; + dmode-vsync_end =
Re: [PATCH 1/2 v6] of: add helper to parse display timings
Hi, On Mon, Oct 08, 2012 at 10:07:45AM +0300, Tomi Valkeinen wrote: Hi, On Thu, 2012-10-04 at 19:59 +0200, Steffen Trumtrar wrote: Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de --- .../devicetree/bindings/video/display-timings.txt | 222 drivers/of/Kconfig |5 + drivers/of/Makefile|1 + drivers/of/of_display_timings.c| 183 include/linux/of_display_timings.h | 85 5 files changed, 496 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt create mode 100644 drivers/of/of_display_timings.c create mode 100644 include/linux/of_display_timings.h diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt new file mode 100644 index 000..45e39bd --- /dev/null +++ b/Documentation/devicetree/bindings/video/display-timings.txt @@ -0,0 +1,222 @@ +display-timings bindings +== + +display-timings-node + + +required properties: + - none + +optional properties: + - default-timing: the default timing value + +timings-subnode +--- + +required properties: + - hactive, vactive: Display resolution + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters + in pixels + vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in + lines + - clock: displayclock in Hz + +optional properties: + - hsync-active-high (bool): Hsync pulse is active high + - vsync-active-high (bool): Vsync pulse is active high + - de-active-high (bool): Data-Enable pulse is active high + - pixelclk-inverted (bool): pixelclock is inverted + - interlaced (bool) + - doublescan (bool) I think bool should be generally used for things that are on/off, like interlace. For hsync-active-high others I'd rather have 0/1 values as others already suggested. +There are different ways of describing the capabilities of a display. The devicetree +representation corresponds to the one commonly found in datasheets for displays. +If a display supports multiple signal timings, the default-timing can be specified. + +The parameters are defined as + +struct signal_timing +=== + + +--+-+--+---+ + | |↑| | | + | ||vback_porch | | | + | |↓| | | + +--###--+---+ + | #↑# | | + | #|# | | + | hback #|# hfront | hsync | + | porch #| hactive # porch | len | + |#---+---#|-| + | #|# | | + | #|vactive # | | + | #|# | | + | #↓# | | + +--###--+---+ + | |↑| | | + | ||vfront_porch| | | + | |↓| | | + +--+-+--+---+ + | |↑| | | + | ||vsync_len | | | + | |↓| | | + +--+-+--+---+ + + +Example: + + display-timings { + default-timing = timing0; + timing0: 1920p24 { + /* 1920x1080p24 */ I think this is commonly called 1080p24. Oops. Yes, you are right. + clock = 5200; + hactive = 1920; + vactive = 1080; + hfront-porch = 25; + hback-porch = 25; + hsync-len = 25; + vback-porch = 2; +
Re: [PATCH 2/2 v6] of: add generic videomode description
On Mon, Oct 08, 2012 at 10:21:53AM +0300, Tomi Valkeinen wrote: On Thu, 2012-10-04 at 19:59 +0200, Steffen Trumtrar wrote: Get videomode from devicetree in a format appropriate for the backend. drm_display_mode and fb_videomode are supported atm. Uses the display signal timings from of_display_timings Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de --- drivers/of/Kconfig |5 + drivers/of/Makefile |1 + drivers/of/of_videomode.c| 212 ++ include/linux/of_videomode.h | 41 4 files changed, 259 insertions(+) create mode 100644 drivers/of/of_videomode.c create mode 100644 include/linux/of_videomode.h diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index 646deb0..74282e2 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -88,4 +88,9 @@ config OF_DISPLAY_TIMINGS help helper to parse display timings from the devicetree +config OF_VIDEOMODE + def_bool y + help + helper to get videomodes from the devicetree + endmenu # OF diff --git a/drivers/of/Makefile b/drivers/of/Makefile index c8e9603..09d556f 100644 --- a/drivers/of/Makefile +++ b/drivers/of/Makefile @@ -12,3 +12,4 @@ obj-$(CONFIG_OF_PCI) += of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o obj-$(CONFIG_OF_MTD) += of_mtd.o obj-$(CONFIG_OF_DISPLAY_TIMINGS) += of_display_timings.o +obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c new file mode 100644 index 000..76ac16e --- /dev/null +++ b/drivers/of/of_videomode.c @@ -0,0 +1,212 @@ +/* + * generic videomode helper + * + * Copyright (c) 2012 Steffen Trumtrar s.trumt...@pengutronix.de, Pengutronix + * + * This file is released under the GPLv2 + */ +#include linux/of.h +#include linux/fb.h +#include linux/slab.h +#include drm/drm_mode.h +#include linux/of_display_timings.h +#include linux/of_videomode.h + +void dump_fb_videomode(struct fb_videomode *m) +{ +pr_debug(fb_videomode = %d %d %d %d %d %d %d %d %d %d %d %d %d\n, +m-refresh, m-xres, m-yres, m-pixclock, m-left_margin, +m-right_margin, m-upper_margin, m-lower_margin, +m-hsync_len, m-vsync_len, m-sync, m-vmode, m-flag); +} + +void dump_drm_displaymode(struct drm_display_mode *m) +{ +pr_debug(drm_displaymode = %d %d %d %d %d %d %d %d %d\n, +m-hdisplay, m-hsync_start, m-hsync_end, m-htotal, +m-vdisplay, m-vsync_start, m-vsync_end, m-vtotal, +m-clock); +} + +int videomode_from_timing(struct display_timings *disp, struct videomode *vm, + int index) +{ + struct signal_timing *st = NULL; + + if (!vm) + return -EINVAL; + + st = display_timings_get(disp, index); + + if (!st) { + pr_err(%s: no signal timings found\n, __func__); + return -EINVAL; + } + + vm-pixelclock = signal_timing_get_value(st-pixelclock, 0); + vm-hactive = signal_timing_get_value(st-hactive, 0); + vm-hfront_porch = signal_timing_get_value(st-hfront_porch, 0); + vm-hback_porch = signal_timing_get_value(st-hback_porch, 0); + vm-hsync_len = signal_timing_get_value(st-hsync_len, 0); + + vm-vactive = signal_timing_get_value(st-vactive, 0); + vm-vfront_porch = signal_timing_get_value(st-vfront_porch, 0); + vm-vback_porch = signal_timing_get_value(st-vback_porch, 0); + vm-vsync_len = signal_timing_get_value(st-vsync_len, 0); + + vm-vah = st-vsync_pol_active_high; + vm-hah = st-hsync_pol_active_high; + vm-interlaced = st-interlaced; + vm-doublescan = st-doublescan; + + return 0; +} + +int of_get_videomode(struct device_node *np, struct videomode *vm, int index) +{ + struct display_timings *disp; + int ret = 0; + + disp = of_get_display_timing_list(np); + + if (!disp) { + pr_err(%s: no timings specified\n, __func__); + return -EINVAL; + } + + if (index == OF_DEFAULT_TIMING) + index = disp-default_timing; + + ret = videomode_from_timing(disp, vm, index); + + if (ret) + return ret; + + display_timings_release(disp); + + if (!vm) { + pr_err(%s: could not get videomode %d\n, __func__, index); + return -EINVAL; + } + + return 0; +} +EXPORT_SYMBOL_GPL(of_get_videomode); + +#if defined(CONFIG_DRM) +int videomode_to_display_mode(struct videomode *vm, struct drm_display_mode *dmode) +{ + memset(dmode, 0, sizeof(*dmode)); + + dmode-hdisplay = vm-hactive; + dmode-hsync_start = dmode-hdisplay + vm-hfront_porch; + dmode-hsync_end = dmode-hsync_start + vm-hsync_len; + dmode-htotal = dmode-hsync_end + vm-hback_porch;
Re: Using Linux 3.2.23, GNOME 3.4 fallback, `evolution --component=mail mailto:i...@example.net` hangs
On Son, 2012-10-07 at 14:50 +0200, Paul Menzel wrote: using Debian Sid/unstable, clicking on a mail address in Iceweasel (Firefox) a composer window is opened in Evolution 3.4.3-1. But then the windows are only gray and hang and the only way to fix this is to kill the process. I reported this issue to the GNOME Bugzilla as 685640 and the developer Matthew Barnes pointed out the following [1]. --- Comment #1 from Matthew Barnes mbar...@redhat.com 2012-10-07 11:59:35 UTC --- Radeon video driver thread looks stuck: Thread 5 (Thread 0x7f6656d4d700 (LWP 4375)): #0 pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:162 No locals. #1 0x7f6656f4c9f3 in pipe_semaphore_wait (sema=0x7f666b6577b8) at ../../../../../src/gallium/auxiliary/os/os_thread.h:432 No locals. #2 radeon_drm_cs_emit_ioctl (param=0x7f666b635c40) at radeon_drm_cs.c:401 cs = 0x7f666b635c40 I followed up on the bug report with an explanation why that isn't necessarily a problem. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer signature.asc Description: This is a digitally signed message part ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv9 18/25] v4l: add buffer exporting via dmabuf
Hi Hans, On 10/07/2012 04:17 PM, Hans Verkuil wrote: On Sun October 7 2012 15:38:30 Laurent Pinchart wrote: Hi Hans, On Friday 05 October 2012 10:55:40 Hans Verkuil wrote: On Tue October 2 2012 16:27:29 Tomasz Stanislawski wrote: This patch adds extension to V4L2 api. It allow to export a mmap buffer as file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by mmap and return a file descriptor on success. Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com [snip] diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index e04a73e..f429b6a 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -688,6 +688,33 @@ struct v4l2_buffer { #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x0800 #define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x1000 +/** + * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor + * + * @fd: file descriptor associated with DMABUF (set by driver) + * @flags:flags for newly created file, currently only O_CLOEXEC is + *supported, refer to manual of open syscall for more details + * @index:id number of the buffer + * @type: enum v4l2_buf_type; buffer type (type == *_MPLANE for + *multiplanar buffers); + * @plane:index of the plane to be exported, 0 for single plane queues + * + * Contains data used for exporting a video buffer as DMABUF file descriptor. + * The buffer is identified by a 'cookie' returned by VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to userspace). All + * reserved fields must be set to zero. The field reserved0 is expected to + * become a structure 'type' allowing an alternative layout of the structure + * content. Therefore this field should not be used for any other extensions. + */ +struct v4l2_exportbuffer { + __s32 fd; + __u32 flags; + __u32 type; /* enum v4l2_buf_type */ + __u32 index; + __u32 plane; As suggested in my comments in the previous patch, I think it is a more natural order to have the type/index/plane fields first in this struct. Actually, I think that flags should also come before fd: struct v4l2_exportbuffer { __u32 type; /* enum v4l2_buf_type */ __u32 index; __u32 plane; __u32 flags; __s32 fd; __u32 reserved[11]; }; It would indeed feel more natural, but putting them right before the reserved fields allows creating an anonymous union around type, index and plane and extending it with reserved fields if needed. That's (at least to my understanding) the rationale behind the current structure layout. The anonymous union argument makes no sense to me, to be honest. I agree that the anonymous unions are not good solutions because they are not supported in many C dialects. However I have nothing against using named unions. It's standard practice within V4L2 to have the IN fields first, then the OUT fields, followed by reserved fields for future expansion. IMO, the input/output/reserved rule is only a recommendation. The are places in V4L2 where this rule is violated with structure v4l2_buffer being the most notable example. Notice that if at least one of the reserved fields becomes an input file then the rule will be violated anyway. Should we ever need a, say, sub-plane index (whatever that might be), then we can use one of the reserved fields. Maybe not subplane :). But maybe some data_offset for exporting only a part of the buffer will be needed some day. Moreover, the integration of DMABUF with the DMA synchronization framework may involve passing additional parameters from the userspace. Notice that flags and fd fields are not logically connected with (type/index/plane) tuple. Therefore both field sets should be separated by some reserved fields to ensure that any of them can be extended if needed. This was the rationale for the structure layout in v9. Regards, Tomasz Stanislawski Regards, Hans + __u32 reserved[11]; +}; + /* *O V E R L A Y P R E V I E W */ ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 50149] Faulty shaders on RS600
https://bugs.freedesktop.org/show_bug.cgi?id=50149 --- Comment #24 from Roman Šmakal schmakeri...@gmail.com --- Torchlight seems to have same rendering issues as seen on HoN video. Time after time textures and models flickers. Playable though .. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv9 18/25] v4l: add buffer exporting via dmabuf
On Mon October 8 2012 11:40:37 Tomasz Stanislawski wrote: Hi Hans, On 10/07/2012 04:17 PM, Hans Verkuil wrote: On Sun October 7 2012 15:38:30 Laurent Pinchart wrote: Hi Hans, On Friday 05 October 2012 10:55:40 Hans Verkuil wrote: On Tue October 2 2012 16:27:29 Tomasz Stanislawski wrote: This patch adds extension to V4L2 api. It allow to export a mmap buffer as file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by mmap and return a file descriptor on success. Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com [snip] diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index e04a73e..f429b6a 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -688,6 +688,33 @@ struct v4l2_buffer { #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x0800 #define V4L2_BUF_FLAG_NO_CACHE_CLEAN0x1000 +/** + * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor + * + * @fd: file descriptor associated with DMABUF (set by driver) + * @flags: flags for newly created file, currently only O_CLOEXEC is + * supported, refer to manual of open syscall for more details + * @index: id number of the buffer + * @type: enum v4l2_buf_type; buffer type (type == *_MPLANE for + * multiplanar buffers); + * @plane: index of the plane to be exported, 0 for single plane queues + * + * Contains data used for exporting a video buffer as DMABUF file descriptor. + * The buffer is identified by a 'cookie' returned by VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to userspace). All + * reserved fields must be set to zero. The field reserved0 is expected to + * become a structure 'type' allowing an alternative layout of the structure + * content. Therefore this field should not be used for any other extensions. + */ +struct v4l2_exportbuffer { +__s32 fd; +__u32 flags; +__u32 type; /* enum v4l2_buf_type */ +__u32 index; +__u32 plane; As suggested in my comments in the previous patch, I think it is a more natural order to have the type/index/plane fields first in this struct. Actually, I think that flags should also come before fd: struct v4l2_exportbuffer { __u32 type; /* enum v4l2_buf_type */ __u32 index; __u32 plane; __u32 flags; __s32 fd; __u32 reserved[11]; }; It would indeed feel more natural, but putting them right before the reserved fields allows creating an anonymous union around type, index and plane and extending it with reserved fields if needed. That's (at least to my understanding) the rationale behind the current structure layout. The anonymous union argument makes no sense to me, to be honest. I agree that the anonymous unions are not good solutions because they are not supported in many C dialects. However I have nothing against using named unions. Named or unnamed, I don't see how a union will help. What do you want to do with a union? It's standard practice within V4L2 to have the IN fields first, then the OUT fields, followed by reserved fields for future expansion. IMO, the input/output/reserved rule is only a recommendation. The are places in V4L2 where this rule is violated with structure v4l2_buffer being the most notable example. Notice that if at least one of the reserved fields becomes an input file then the rule will be violated anyway. Sure, but there is no legacy yet, so why not keep to the recommendation? Should we ever need a, say, sub-plane index (whatever that might be), then we can use one of the reserved fields. Maybe not subplane :). But maybe some data_offset for exporting only a part of the buffer will be needed some day. Moreover, the integration of DMABUF with the DMA synchronization framework may involve passing additional parameters from the userspace. Notice that flags and fd fields are not logically connected with (type/index/plane) tuple. Therefore both field sets should be separated by some reserved fields to ensure that any of them can be extended if needed. This was the rationale for the structure layout in v9. It's a bad idea to add multiple 'reserved' arrays, that makes userspace harder since it has to zero all of them instead of just one. Actually, the same applies to kernel space, which has to zero them as well. I still don't know why you want to use a non-standard field order. Regards, Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: allocate page tables on demand v3
Got time today to test that a bit more, no piglit regressions on SI so far. And also CAYMAN still seems to work with it, but there are allot of piglit tests there that still doesn't work when enabling VM (and it doesn't matter if you have this patch or not). Any objections to pull that into 3.7 as well? Cheers, Christian. On 07.10.2012 15:06, Christian König wrote: Based on Dmitries work, but splitting the code into page directory and page table handling makes it far more readable and (hopefully) more reliable. Allocations of page tables are made from the SA on demand, that should still work fine since all page tables are of the same size. Also using the fact that allocations from the SA are mostly continuously (except for end of buffer wraps and under very high memory pressure) to group updates send to the chipset specific code into larger chunks. v3: mostly a rewrite of Dmitries previous patch. Signed-off-by: Dmitry Cherkasov dmitrii.cherka...@amd.com Signed-off-by: Christian König deathsim...@vodafone.de --- drivers/gpu/drm/radeon/ni.c |2 +- drivers/gpu/drm/radeon/radeon.h | 11 +- drivers/gpu/drm/radeon/radeon_gart.c | 323 ++ 3 files changed, 263 insertions(+), 73 deletions(-) diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c index 9a46f7d..48e2337 100644 --- a/drivers/gpu/drm/radeon/ni.c +++ b/drivers/gpu/drm/radeon/ni.c @@ -1576,7 +1576,7 @@ void cayman_vm_flush(struct radeon_device *rdev, int ridx, struct radeon_vm *vm) radeon_ring_write(ring, 0); radeon_ring_write(ring, PACKET0(VM_CONTEXT0_PAGE_TABLE_END_ADDR + (vm-id 2), 0)); - radeon_ring_write(ring, vm-last_pfn); + radeon_ring_write(ring, rdev-vm_manager.max_pfn); radeon_ring_write(ring, PACKET0(VM_CONTEXT0_PAGE_TABLE_BASE_ADDR + (vm-id 2), 0)); radeon_ring_write(ring, vm-pd_gpu_addr 12); diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index b04c064..bc6b56b 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -663,9 +663,14 @@ struct radeon_vm { struct list_headlist; struct list_headva; unsignedid; - unsignedlast_pfn; - u64 pd_gpu_addr; - struct radeon_sa_bo *sa_bo; + + /* contains the page directory */ + struct radeon_sa_bo *page_directory; + uint64_tpd_gpu_addr; + + /* array of page tables, one for each page directory entry */ + struct radeon_sa_bo **page_tables; + struct mutexmutex; /* last fence for cs using this vm */ struct radeon_fence *fence; diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index 753b7ca..40088b0 100644 --- a/drivers/gpu/drm/radeon/radeon_gart.c +++ b/drivers/gpu/drm/radeon/radeon_gart.c @@ -423,6 +423,18 @@ void radeon_gart_fini(struct radeon_device *rdev) */ /** + * radeon_vm_num_pde - return the number of page directory entries + * + * @rdev: radeon_device pointer + * + * Calculate the number of page directory entries (cayman+). + */ +static unsigned radeon_vm_num_pdes(struct radeon_device *rdev) +{ + return (rdev-vm_manager.max_pfn RADEON_VM_BLOCK_SIZE); +} + +/** * radeon_vm_directory_size - returns the size of the page directory in bytes * * @rdev: radeon_device pointer @@ -431,7 +443,7 @@ void radeon_gart_fini(struct radeon_device *rdev) */ static unsigned radeon_vm_directory_size(struct radeon_device *rdev) { - return (rdev-vm_manager.max_pfn RADEON_VM_BLOCK_SIZE) * 8; + return RADEON_GPU_PAGE_ALIGN(radeon_vm_num_pdes(rdev) * 8); } /** @@ -451,11 +463,11 @@ int radeon_vm_manager_init(struct radeon_device *rdev) if (!rdev-vm_manager.enabled) { /* allocate enough for 2 full VM pts */ - size = RADEON_GPU_PAGE_ALIGN(radeon_vm_directory_size(rdev)); - size += RADEON_GPU_PAGE_ALIGN(rdev-vm_manager.max_pfn * 8); + size = radeon_vm_directory_size(rdev); + size += rdev-vm_manager.max_pfn * 8; size *= 2; r = radeon_sa_bo_manager_init(rdev, rdev-vm_manager.sa_manager, - size, + RADEON_GPU_PAGE_ALIGN(size), RADEON_GEM_DOMAIN_VRAM); if (r) { dev_err(rdev-dev, failed to allocate vm bo (%dKB)\n, @@ -476,7 +488,7 @@ int radeon_vm_manager_init(struct radeon_device *rdev) /* restore page table */ list_for_each_entry(vm, rdev-vm_manager.lru_vm, list) { - if (vm-sa_bo == NULL) + if (vm-page_directory == NULL)
Re: [PATCHv9 18/25] v4l: add buffer exporting via dmabuf
Hi Hans, On 10/08/2012 11:54 AM, Hans Verkuil wrote: On Mon October 8 2012 11:40:37 Tomasz Stanislawski wrote: Hi Hans, On 10/07/2012 04:17 PM, Hans Verkuil wrote: On Sun October 7 2012 15:38:30 Laurent Pinchart wrote: Hi Hans, On Friday 05 October 2012 10:55:40 Hans Verkuil wrote: On Tue October 2 2012 16:27:29 Tomasz Stanislawski wrote: This patch adds extension to V4L2 api. It allow to export a mmap buffer as file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by mmap and return a file descriptor on success. Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com [snip] +struct v4l2_exportbuffer { +__s32 fd; +__u32 flags; +__u32 type; /* enum v4l2_buf_type */ +__u32 index; +__u32 plane; As suggested in my comments in the previous patch, I think it is a more natural order to have the type/index/plane fields first in this struct. Actually, I think that flags should also come before fd: struct v4l2_exportbuffer { __u32 type; /* enum v4l2_buf_type */ __u32 index; __u32 plane; __u32 flags; __s32 fd; __u32 reserved[11]; }; It would indeed feel more natural, but putting them right before the reserved fields allows creating an anonymous union around type, index and plane and extending it with reserved fields if needed. That's (at least to my understanding) the rationale behind the current structure layout. The anonymous union argument makes no sense to me, to be honest. I agree that the anonymous unions are not good solutions because they are not supported in many C dialects. However I have nothing against using named unions. Named or unnamed, I don't see how a union will help. What do you want to do with a union? Currently, there exist three sane layouts of the structure, that use only one reserved field: A) struct v4l2_exportbuffer { __s32 fd; __u32 flags; __u32 type; /* enum v4l2_buf_type */ __u32 index; __u32 plane; __u32 reserved[11]; } B) struct v4l2_exportbuffer { __u32 type; /* enum v4l2_buf_type */ __u32 index; __u32 plane; __u32 flags; __s32 fd; __u32 reserved[11]; } C) struct v4l2_exportbuffer { __u32 type; /* enum v4l2_buf_type */ __u32 index; __u32 plane; __u32 reserved[11]; __u32 flags; __s32 fd; } Only the layout B follows 'input/output/reserved' rule. The layouts A and C allows to extend (type/index/plane) tuple without mixing it with (flags,fd). For layouts A and C it is possible to use unions to provide new means of describing a buffer to be exported. struct v4l2_exportbuffer { __s32 fd; __u32 flags; union { struct by_tip { /* type, index, plane */ __u32 type; /* enum v4l2_buf_type */ __u32 index; __u32 plane; } by_tip; struct by_userptr { u64 userptr; u64 length; } by_userptr; __u32 reserved[6]; } b; __u32 union_type; /* BY_TIP or BY_USERPTR */ __u32 reserved[4]; }; No such an extension can be applied for layout B. The similar scheme can be used for layout C. Moreover it support extensions and variants for (flags/fd) tuple. It might be useful if one day we would like to export a buffer as something different from DMABUF file descriptors. Anyway, we have to choose between the elegance of the layout and the extensibility. I think that layout A is a good trade-off. We could swap fd and flags to get little closer to the rule. It's standard practice within V4L2 to have the IN fields first, then the OUT fields, followed by reserved fields for future expansion. IMO, the input/output/reserved rule is only a recommendation. The are places in V4L2 where this rule is violated with structure v4l2_buffer being the most notable example. Notice that if at least one of the reserved fields becomes an input file then the rule will be violated anyway. Sure, but there is no legacy yet, so why not keep to the recommendation? Should we ever need a, say, sub-plane index (whatever that might be), then we can use one of the reserved fields. Maybe not subplane :). But maybe some data_offset for exporting only a part of the buffer will be needed some day. Moreover, the integration of DMABUF with the DMA synchronization framework may involve passing
Re: [PATCHv9 18/25] v4l: add buffer exporting via dmabuf
On Mon October 8 2012 12:41:28 Tomasz Stanislawski wrote: Hi Hans, On 10/08/2012 11:54 AM, Hans Verkuil wrote: On Mon October 8 2012 11:40:37 Tomasz Stanislawski wrote: Hi Hans, On 10/07/2012 04:17 PM, Hans Verkuil wrote: On Sun October 7 2012 15:38:30 Laurent Pinchart wrote: Hi Hans, On Friday 05 October 2012 10:55:40 Hans Verkuil wrote: On Tue October 2 2012 16:27:29 Tomasz Stanislawski wrote: This patch adds extension to V4L2 api. It allow to export a mmap buffer as file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by mmap and return a file descriptor on success. Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com [snip] +struct v4l2_exportbuffer { + __s32 fd; + __u32 flags; + __u32 type; /* enum v4l2_buf_type */ + __u32 index; + __u32 plane; As suggested in my comments in the previous patch, I think it is a more natural order to have the type/index/plane fields first in this struct. Actually, I think that flags should also come before fd: struct v4l2_exportbuffer { __u32 type; /* enum v4l2_buf_type */ __u32 index; __u32 plane; __u32 flags; __s32 fd; __u32 reserved[11]; }; It would indeed feel more natural, but putting them right before the reserved fields allows creating an anonymous union around type, index and plane and extending it with reserved fields if needed. That's (at least to my understanding) the rationale behind the current structure layout. The anonymous union argument makes no sense to me, to be honest. I agree that the anonymous unions are not good solutions because they are not supported in many C dialects. However I have nothing against using named unions. Named or unnamed, I don't see how a union will help. What do you want to do with a union? Currently, there exist three sane layouts of the structure, that use only one reserved field: A) struct v4l2_exportbuffer { __s32 fd; __u32 flags; __u32 type; /* enum v4l2_buf_type */ __u32 index; __u32 plane; __u32 reserved[11]; } B) struct v4l2_exportbuffer { __u32 type; /* enum v4l2_buf_type */ __u32 index; __u32 plane; __u32 flags; __s32 fd; __u32 reserved[11]; } C) struct v4l2_exportbuffer { __u32 type; /* enum v4l2_buf_type */ __u32 index; __u32 plane; __u32 reserved[11]; __u32 flags; __s32 fd; } Only the layout B follows 'input/output/reserved' rule. The layouts A and C allows to extend (type/index/plane) tuple without mixing it with (flags,fd). For layouts A and C it is possible to use unions to provide new means of describing a buffer to be exported. struct v4l2_exportbuffer { __s32 fd; __u32 flags; union { struct by_tip { /* type, index, plane */ __u32 type; /* enum v4l2_buf_type */ __u32 index; __u32 plane; } by_tip; struct by_userptr { u64 userptr; u64 length; } by_userptr; __u32 reserved[6]; } b; __u32 union_type; /* BY_TIP or BY_USERPTR */ __u32 reserved[4]; }; No such an extension can be applied for layout B. You are overengineering. If we ever want to export something that is *not* a buffer created with REQBUFS/CREATE_BUFS, then you want to do that with a new ioctl. If we want for some reason to export a userptr, then that should either be from a queued/prepared buffer, or we need a separate API to create a dmabuf from a userptr. After all, that would be completely generic and not V4L2 specific. Anyway, introducing such a union later won't work either because then instead of writing expbuf.type you'd have to write expbuf.by_tip.type: API change. BTW, I think it makes sense as well in the case of a userptr to only export the buffer if it is under control of the kernel (e.g. after QBUF or PREPARE_BUF). When exporting the buffer the driver has all the information it needs to do so safely. The similar scheme can be used for layout C. Moreover it support extensions and variants for (flags/fd) tuple. It might be useful if one day we would like to export a buffer as something different from DMABUF file descriptors. Anyway, we have to choose between the elegance of the layout and the extensibility. I
Re: [RFC 0/4] drm: add raw monotonic timestamp support
On Fri, 2012-10-05 at 16:07 -0700, Eric Anholt wrote: Imre Deak imre.d...@intel.com writes: This is needed to make applications depending on vblank/page flip timestamps independent of time ajdustments. I've tested these with an updated intel-gpu-test/flip_test and will send the update for that once there's no objection about this patchset. The patchset is based on danvet's dinq branch with the following additional patches from the intel-gfx ML applied: drm/i915: paper over a pipe-enable vs pageflip race drm/i915: don't frob the vblank ts in finish_page_flip drm/i915: call drm_handle_vblank before finish_page_flip While people are in pageflip code: It would be really, really cool for application tracing if we could get timestamps out of our swaps that used the TIMESTAMP register that is the timer used for event tracing on the GPU using GL_ARB_timer_query. Then you could get decent visualizations of the latency of your rendering. I assume this querying wouldn't be done through the wait_for_vblank or page_flip ioctls, but rather a new ioctl or even through a new perf event? We could add such a new interface later; this patchset is more focused on the above two ioctls. --Imre ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 1/4] time: export getnstime_raw_and_real for DRM
On Sat, 2012-10-06 at 03:41 +0200, Mario Kleiner wrote: [...] But then Psychtoolbox checks each timestamp it gets from somewhere outside (OML_sync_control / INTEL_swap_events / ALSA audio timestamps, network receive timestamps, evdev, x11, ...) if it is in gettimeofday() aka CLOCK_REALTIME aka wall time or in CLOCK_MONOTONIC time and just remaps to whatever its reference clock is. There's no way around this than to have no fixed expectations, but to remap stuff on the fly, because different parts of the Linux universe have decided on different time bases, or even switched somewhere from one kernel version to the next in the last years, e.g., ALSA, which at some time switched from wall clock to CLOCK_MONOTONIC. Sometimes clock_gettime() wasn't available at all in older setups, so there only was gettimeofday(). Or toolkits like GStreamer use different timebases dependent on OS and sometimes even on plugins. I would expect that other timing sensitive apps have to have ways to handle this in similar ways. I think the question is, can we be sure? I don't think there is any guarantee that random application X will not be confused by an unconditional switch to monotonic timestamps. Wrt. to the drm vblank/pageflip timestamps, the userspace extensions which expose these (INTEL_swap_events, OML_sync_control) don't allow apps to select which timebase to use, they define monotonic time as what is returned, so i don't know how a userspace app could actually ask the DRM for one or the other format? So i guess just switching to CLOCK_MONOTONIC shouldn't be that bad. An application could just use the kernel DRM interface directly. I admit this is not very likely but this is what should determine the rules by which we change the ABI. Kristian, i assume Wayland will also return presentation timestamps in the format and microsecond precision of the DRM, right? On 05.10.12 18:22, intel-gfx-requ...@lists.freedesktop.org wrote: Message: 7 Date: Fri, 5 Oct 2012 12:14:29 -0400 From: Kristian H?gsberg ... I just had a quick look at driver/input/evdev.c, since evdev devices did a similar change recently to allow evdev timestamp from the monotonic clock. They're using a different time API: time_mono = ktime_get(); time_real = ktime_sub(time_mono, ktime_get_monotonic_offset()); and event-time = ktime_to_timeval(client-clkid == CLOCK_MONOTONIC ? mono : real); I'm not really up-to-date on kernel time APIs, but I wonder if that may be better? At least, I suspect we wouldn't need changes outside drm if we use this API. Kristian Userspace apps only have access to what gettimeofday() and clock_gettime() for CLOCK_REALTIME (== gettimeofday() afaik) and CLOCK_MONOTONIC return, so whatever is returned should be in CLOCK_MONOTONIC format, otherwise there will be lots of tears and dead kittens. I think what evdev does makes a lot of sense, but i'm also not up-to-date about the various layers of timing apis. Yes, this should be the case, regardless of which kernel interface we decide to use. --Imre ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] libdrm,intel: Add Android makefiles (v2)
On Mon, Oct 08, 2012 at 08:50:25AM +0300, Tapani Pälli wrote: From: Chad Versace chad.vers...@linux.intel.com This enables libdrm.so and libdrm_intel.so to build on Android IceCreamSandwich. v2: Link libdrm_intel to libpciaccess. Signed-off-by: Chad Versace chad.vers...@linux.intel.com Reviewed-by: Oliver McFadden oliver.mcfad...@linux.intel.com --- Android.mk | 52 +++ intel/Android.mk | 57 2 files changed, 109 insertions(+) create mode 100644 Android.mk create mode 100644 intel/Android.mk diff --git a/Android.mk b/Android.mk new file mode 100644 index 000..1029dc6 --- /dev/null +++ b/Android.mk @@ -0,0 +1,52 @@ +# +# Copyright © 2011 Intel Corporation +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the Software), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice (including the next +# paragraph) shall be included in all copies or substantial portions of the +# Software. +# +# THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +# IN THE SOFTWARE. +# + +LIBDRM_VALID_GPU_DRIVERS := i915 i965 + +# Skip this makefile if we're not building for a known GPU. +ifneq ($(filter $(BOARD_GPU_DRIVERS), $(LIBDRM_VALID_GPU_DRIVERS)),) + +LOCAL_PATH := $(call my-dir) +include $(CLEAR_VARS) + +LIBDRM_TOP := $(LOCAL_PATH) + +# Import variable LIBDRM_SOURCES. +include $(LOCAL_PATH)/sources.mk + +LOCAL_MODULE := libdrm +LOCAL_MODULE_TAGS := optional + +LOCAL_SRC_FILES := $(LIBDRM_SOURCES) + +LOCAL_C_INCLUDES := \ + $(LIBDRM_TOP)/include/drm + +LOCAL_CFLAGS := \ + -DHAVE_LIBDRM_ATOMIC_PRIMITIVES=1 + +include $(BUILD_SHARED_LIBRARY) + +include $(LOCAL_PATH)/intel/Android.mk + +endif # BOARD_GPU_DRIVERS diff --git a/intel/Android.mk b/intel/Android.mk new file mode 100644 index 000..3647a14 --- /dev/null +++ b/intel/Android.mk @@ -0,0 +1,57 @@ +# +# Copyright © 2011 Intel Corporation +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the Software), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice (including the next +# paragraph) shall be included in all copies or substantial portions of the +# Software. +# +# THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +# IN THE SOFTWARE. +# + +LIBDRM_INTEL_VALID_GPU_DRIVERS := i915 i965 + +# Skip this makefile if we're not building for an Intel GPU. +ifneq ($(filter $(BOARD_GPU_DRIVERS), $(LIBDRM_INTEL_VALID_GPU_DRIVERS)),) + +LOCAL_PATH := $(call my-dir) +include $(CLEAR_VARS) + +# Import variable LIBDRM_INTEL_SOURCES. +include $(LOCAL_PATH)/sources.mk + +LOCAL_MODULE := libdrm_intel +LOCAL_MODULE_TAGS := optional + +LOCAL_SHARED_LIBRARIES := libdrm + +LOCAL_SRC_FILES := $(LIBDRM_INTEL_SOURCES) + +LOCAL_C_INCLUDES := \ + $(LIBDRM_TOP) \ + $(LIBDRM_TOP)/intel \ + $(LIBDRM_TOP)/include/drm \ + external/libpciaccess/include + +LOCAL_CFLAGS := \ + -DHAVE_LIBDRM_ATOMIC_PRIMITIVES=1 + +LOCAL_SHARED_LIBRARIES := \ + libdrm \ + libpciaccess + +include $(BUILD_SHARED_LIBRARY) + +endif # BOARD_GPU_DRIVERS -- 1.7.11.7 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Oliver McFadden.
Re: [PATCH] Android.mk: use LOCAL_COPY_HEADERS to export headers. v2
On Mon, Oct 08, 2012 at 12:23:56PM +0300, Tapani Pälli wrote: From: Haitao Huang haitao.hu...@intel.com Export necessary header files used by other components for Android, such as libva intel-driver, gralloc, hwcomposer, etc. Signed-off-by: Haitao Huang haitao.hu...@intel.com [chad: Fixed inconsistent indentation.] Signed-off-by: Chad Versace chad.vers...@linux.intel.com --- Normal syntax is Subject: [PATCH v2 N/M] ... You should add a comment here describing the v1..v2 diff. Android.mk | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Android.mk b/Android.mk index 1029dc6..68db5df 100644 --- a/Android.mk +++ b/Android.mk @@ -1,5 +1,5 @@ # -# Copyright © 2011 Intel Corporation +# Copyright © 2011-2012 Intel Corporation # # Permission is hereby granted, free of charge, to any person obtaining a # copy of this software and associated documentation files (the Software), @@ -45,6 +45,16 @@ LOCAL_C_INCLUDES := \ LOCAL_CFLAGS := \ -DHAVE_LIBDRM_ATOMIC_PRIMITIVES=1 +LOCAL_COPY_HEADERS :=\ + xf86drm.h\ + include/drm/drm_fourcc.h \ + include/drm/drm.h\ + include/drm/drm_mode.h \ + include/drm/drm_sarea.h \ + include/drm/i915_drm.h \ + intel/intel_bufmgr.h \ + +LOCAL_COPY_HEADERS_TO := libdrm include $(BUILD_SHARED_LIBRARY) include $(LOCAL_PATH)/intel/Android.mk -- 1.7.11.7 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Oliver McFadden. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 0/4] drm: add raw monotonic timestamp support (Imre Deak)
On Sat, 2012-10-06 at 04:46 +0200, Mario Kleiner wrote: On 05.10.12 15:37, intel-gfx-requ...@lists.freedesktop.org wrote: Today's Topics: 1. [RFC 0/4] drm: add raw monotonic timestamp support (Imre Deak) 2. [RFC 1/4] time: export getnstime_raw_and_real for DRM (Imre Deak) 3. [RFC 2/4] drm: make memset/calloc for _vblank_time more robust (Imre Deak) 4. [RFC 3/4] drm: use raw time in drm_calc_vbltimestamp_from_scanoutpos (Imre Deak) 5. [RFC 4/4] drm: add support for raw monotonic vblank timestamps (Imre Deak) -- Message: 1 Date: Fri, 5 Oct 2012 16:36:58 +0300 From: Imre Deakimre.d...@intel.com To: Daniel Vetterdaniel.vet...@ffwll.ch, Chris Wilson ch...@chris-wilson.co.uk, Kristian H?gsbergk...@bitplanet.net Cc:intel-...@lists.freedesktop.org,dri-devel@lists.freedesktop.org Subject: [Intel-gfx] [RFC 0/4] drm: add raw monotonic timestamp support Message-ID:1349444222-22274-1-git-send-email-imre.d...@intel.com This is needed to make applications depending on vblank/page flip timestamps independent of time ajdustments. I've tested these with an updated intel-gpu-test/flip_test and will send the update for that once there's no objection about this patchset. I'm mostly fine with this, although the wall time compatibility stuff may not be useful given that userspace apps, e.g., OpenGL clients, have no way to actually ask for wall vs. monotonic, and the spec actually expects them to expect monotonic timestamps. I also see that an update to nouveau-kms is missing? Afaik the vblank timestamping on nouveau-kms is still handled by the fallback in the drm, but pageflip completion uses do_gettimeofday() for the timestamping and returns a hard-coded zero vblank count all time for pageflip events (yay!). Lucas Stach and me have written and tested some patches to fix this over a year ago, but somehow they never made it into the kms driver, mostly due to bad timing in when stuff was submitted, reviewed and revised, not for serious technical objections iirc. Ideally we could give them another try, or at least update nouveaus pageflip timestamping to avoid really bad breakage for OpenGL clients or the nouveau-ddx due to inconsistent vblank vs. pageflip timestamps. I quickly looked over the patches, i think they look mostly good, see some small comments below. Subject: [Intel-gfx] [RFC 3/4] drm: use raw time in drm_calc_vbltimestamp_from_scanoutpos Message-ID: 1349444222-22274-4-git-send-email-imre.d...@intel.com The timestamp is used here for handling the timeout case, so we don't want it to be affected by time adjustments. Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/drm_irq.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 77f6577..5e42981 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -576,7 +576,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc, unsigned flags, struct drm_crtc *refcrtc) { - struct timeval stime, raw_time; + struct timespec raw_stime, raw_etime, real_etime; struct drm_display_mode *mode; int vbl_status, vtotal, vdisplay; int vpos, hpos, i; @@ -625,13 +625,13 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc, preempt_disable(); /* Get system timestamp before query. */ - do_gettimeofday(stime); + getrawmonotonic(raw_stime); /* Get vertical and horizontal scanout pos. vpos, hpos. */ vbl_status = dev-driver-get_scanout_position(dev, crtc, vpos, hpos); /* Get system timestamp after query. */ - do_gettimeofday(raw_time); + getnstime_raw_and_real(raw_etime, real_etime); preempt_enable(); @@ -642,7 +642,8 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc, return -EIO; } - duration_ns = timeval_to_ns(raw_time) - timeval_to_ns(stime); + duration_ns = timespec_to_ns(raw_etime) - + timespec_to_ns(raw_stime); /* Accept result with max_error nsecs timing uncertainty. */ if (duration_ns = (s64) *max_error) @@ -692,11 +693,11 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc, /* Subtract time delta from raw timestamp to get final * vblank_time timestamp for end of vblank. */ - *vblank_time = ns_to_timeval(timeval_to_ns(raw_time) - delta_ns); + *vblank_time = ns_to_timeval(timeval_to_ns(real_time) -
Re: [PATCH 1/2 v6] of: add helper to parse display timings
Hi Tomi, On Monday 08 October 2012 12:01:18 Tomi Valkeinen wrote: On Mon, 2012-10-08 at 10:25 +0200, Guennadi Liakhovetski wrote: In general, I might be misunderstanding something, but don't we have to distinguish between 2 types of information about display timings: (1) is defined by the display controller requirements, is known to the display driver and doesn't need to be present in timings DT. We did have some of these parameters in board data previously, because we didn't have proper display controller drivers... (2) is board specific configuration, and is such it has to be present in DT. In that way, doesn't interlaced belong to type (1) and thus doesn't need to be present in DT? As I see it, this DT data is about the display (most commonly LCD panel), i.e. what video mode(s) the panel supports. If things were done my way, the panel's supported timings would be defined in the driver for the panel, and DT would be left to describe board specific data, but this approach has its benefits. What about dumb DPI panels ? They will all be supported by a single driver, would you have the driver contain information about all known DPI panels ? DT seems a good solution to me in this case. For complex panels where the driver will support a single (or very few) model I agree that specifying the timings in DT isn't needed. Thus, if you connect an interlaced panel to your board, you need to tell the display controller that this panel requires interlace signal. Also, pixel clock source doesn't make sense in this context, as this doesn't describe the actual used configuration, but only what the panel supports. Of course, if this is about describing the hardware, the default-mode property doesn't really fit in... Maybe we should rename it to native-mode then ? -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2 v6] of: add generic videomode description
Hi Steffen, Thanks for the patch. On Thursday 04 October 2012 19:59:20 Steffen Trumtrar wrote: Get videomode from devicetree in a format appropriate for the backend. drm_display_mode and fb_videomode are supported atm. Uses the display signal timings from of_display_timings Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de --- drivers/of/Kconfig |5 + drivers/of/Makefile |1 + drivers/of/of_videomode.c| 212 +++ include/linux/of_videomode.h | 41 4 files changed, 259 insertions(+) create mode 100644 drivers/of/of_videomode.c create mode 100644 include/linux/of_videomode.h diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index 646deb0..74282e2 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -88,4 +88,9 @@ config OF_DISPLAY_TIMINGS help helper to parse display timings from the devicetree +config OF_VIDEOMODE + def_bool y + help + helper to get videomodes from the devicetree + endmenu # OF diff --git a/drivers/of/Makefile b/drivers/of/Makefile index c8e9603..09d556f 100644 --- a/drivers/of/Makefile +++ b/drivers/of/Makefile @@ -12,3 +12,4 @@ obj-$(CONFIG_OF_PCI)+= of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o obj-$(CONFIG_OF_MTD) += of_mtd.o obj-$(CONFIG_OF_DISPLAY_TIMINGS) += of_display_timings.o +obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c new file mode 100644 index 000..76ac16e --- /dev/null +++ b/drivers/of/of_videomode.c @@ -0,0 +1,212 @@ +/* + * generic videomode helper + * + * Copyright (c) 2012 Steffen Trumtrar s.trumt...@pengutronix.de, Pengutronix + * + * This file is released under the GPLv2 + */ +#include linux/of.h +#include linux/fb.h +#include linux/slab.h +#include drm/drm_mode.h +#include linux/of_display_timings.h +#include linux/of_videomode.h + +void dump_fb_videomode(struct fb_videomode *m) +{ +pr_debug(fb_videomode = %d %d %d %d %d %d %d %d %d %d %d %d %d\n, That's going to be pretty difficult to read :-) Would it make sense to group several attributes logically (for instance using %ux%u for m-xres, m-yres) ? +m-refresh, m-xres, m-yres, m-pixclock, m-left_margin, +m-right_margin, m-upper_margin, m-lower_margin, + m-hsync_len, m-vsync_len, m-sync, m-vmode, m-flag); +} Shouldn't this (and the other non exported functions below) be static ? +void dump_drm_displaymode(struct drm_display_mode *m) +{ +pr_debug(drm_displaymode = %d %d %d %d %d %d %d %d %d\n, +m-hdisplay, m-hsync_start, m-hsync_end, m-htotal, +m-vdisplay, m-vsync_start, m-vsync_end, m-vtotal, +m-clock); +} + +int videomode_from_timing(struct display_timings *disp, struct videomode *vm, + int index) +{ + struct signal_timing *st = NULL; + + if (!vm) + return -EINVAL; + What about making vm a mandatory argument ? It looks to me like a caller bug if vm is NULL. + st = display_timings_get(disp, index); + You can remove the blank line. + if (!st) { + pr_err(%s: no signal timings found\n, __func__); + return -EINVAL; + } + + vm-pixelclock = signal_timing_get_value(st-pixelclock, 0); + vm-hactive = signal_timing_get_value(st-hactive, 0); + vm-hfront_porch = signal_timing_get_value(st-hfront_porch, 0); + vm-hback_porch = signal_timing_get_value(st-hback_porch, 0); + vm-hsync_len = signal_timing_get_value(st-hsync_len, 0); + + vm-vactive = signal_timing_get_value(st-vactive, 0); + vm-vfront_porch = signal_timing_get_value(st-vfront_porch, 0); + vm-vback_porch = signal_timing_get_value(st-vback_porch, 0); + vm-vsync_len = signal_timing_get_value(st-vsync_len, 0); + + vm-vah = st-vsync_pol_active_high; + vm-hah = st-hsync_pol_active_high; + vm-interlaced = st-interlaced; + vm-doublescan = st-doublescan; + + return 0; +} + +int of_get_videomode(struct device_node *np, struct videomode *vm, int index) I wonder how to avoid abuse of this functions. It's a useful helper for drivers that need to get a video mode once only, but would result in lower performances if a driver calls it for every mode. Drivers must call of_get_display_timing_list instead in that case and case the display timings. I'm wondering whether we should really expose of_get_videomode. +{ + struct display_timings *disp; + int ret = 0; No need to assign ret to 0 here. + + disp = of_get_display_timing_list(np); + You can remove the blank line. + if (!disp) { + pr_err(%s: no timings specified\n, __func__); + return -EINVAL; + } + + if (index == OF_DEFAULT_TIMING) + index = disp-default_timing; + + ret =
Re: [PATCH 2/2 v6] of: add generic videomode description
Hi Steffen, On Monday 08 October 2012 09:57:41 Steffen Trumtrar wrote: On Mon, Oct 08, 2012 at 10:21:53AM +0300, Tomi Valkeinen wrote: On Thu, 2012-10-04 at 19:59 +0200, Steffen Trumtrar wrote: [snip] diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h new file mode 100644 index 000..96efe01 --- /dev/null +++ b/include/linux/of_videomode.h @@ -0,0 +1,41 @@ +/* + * Copyright 2012 Steffen Trumtrar s.trumt...@pengutronix.de + * + * generic videomode description + * + * This file is released under the GPLv2 + */ + +#ifndef __LINUX_VIDEOMODE_H +#define __LINUX_VIDEOMODE_H + +#include drm/drmP.h You don't need to include this. That is a fix to my liking. Easily done ;-) +struct videomode { + u32 pixelclock; + u32 refreshrate; + + u32 hactive; + u32 hfront_porch; + u32 hback_porch; + u32 hsync_len; + + u32 vactive; + u32 vfront_porch; + u32 vback_porch; + u32 vsync_len; + + bool hah; + bool vah; + bool interlaced; + bool doublescan; + +}; This is not really of related. And actually, neither is the struct signal_timing in the previous patch. It would be nice to have these in a common header that fb, drm, and others could use instead of each having their own timing structs. But that's probably out of scope for this series =). Did you check the timing structs from the video related frameworks in the kernel to see if your structs contain all the info the others have, so that, at least in theory, everybody could use these common structs? Tomi Yes. Stephen and Laurent already suggested to split it up. No, all info is not contained. That starts with drm, which has width-mm,.. If time permits, I will go over that. Just to make sure we won't forget it, the V4L2 version of the timings structure is struct v4l2_bt_timings in include/linux/videodev2.h. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2 v6] of: add helper to parse display timings
On Mon, 2012-10-08 at 14:04 +0200, Laurent Pinchart wrote: Hi Tomi, On Monday 08 October 2012 12:01:18 Tomi Valkeinen wrote: On Mon, 2012-10-08 at 10:25 +0200, Guennadi Liakhovetski wrote: In general, I might be misunderstanding something, but don't we have to distinguish between 2 types of information about display timings: (1) is defined by the display controller requirements, is known to the display driver and doesn't need to be present in timings DT. We did have some of these parameters in board data previously, because we didn't have proper display controller drivers... (2) is board specific configuration, and is such it has to be present in DT. In that way, doesn't interlaced belong to type (1) and thus doesn't need to be present in DT? As I see it, this DT data is about the display (most commonly LCD panel), i.e. what video mode(s) the panel supports. If things were done my way, the panel's supported timings would be defined in the driver for the panel, and DT would be left to describe board specific data, but this approach has its benefits. What about dumb DPI panels ? They will all be supported by a single driver, would you have the driver contain information about all known DPI panels ? DT seems a good solution to me in this case. Yes, I would have a table in the driver for all the devices it supports, which would describe the device specific parameters. But I don't have a problem with DT solution. Both methods have their pros and cons, and perhaps DT based solution is more practical. For complex panels where the driver will support a single (or very few) model I agree that specifying the timings in DT isn't needed. Thus, if you connect an interlaced panel to your board, you need to tell the display controller that this panel requires interlace signal. Also, pixel clock source doesn't make sense in this context, as this doesn't describe the actual used configuration, but only what the panel supports. Of course, if this is about describing the hardware, the default-mode property doesn't really fit in... Maybe we should rename it to native-mode then ? Hmm, right, if it means native mode, then it is describing the hardware. But would it make sense to require that the native mode is the first mode in the list, then? This would make the separate default-mode/native-mode property not needed. Tomi signature.asc Description: This is a digitally signed message part ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] gma500: medfield: fix potential NULL pointer dereference in mdfld_dsi_output_init()
On Sun, 7 Oct 2012 21:56:45 +0800 Wei Yongjun weiyj...@gmail.com wrote: From: Wei Yongjun yongjun_...@trendmicro.com.cn The dereference should be moved below the NULL test. The !dev check just wants removing I think - it's a bogus check in the first place. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel