RE: [PATCH v2] DA8XX/OMAP-L1XX: FB: Implement double buffering
On Wed, Mar 31, 2010 at 20:43:29, Ambrose, Martin wrote:
Andrew,
Thanks for the feedback.
+/*
+ * Function to wait for vertical sync which for this LCD peripheral
+ * translates into waiting for the current raster frame to complete.
+ */
+static int fb_wait_for_vsync(struct fb_info *info)
+{
+ struct da8xx_fb_par *par = info-par;
+ wait_queue_t wq;
+ int ret;
+
+ init_waitqueue_entry(wq, current);
DECLARE_WAITQUEUE() would be more conventional.
I'll update -- cloned this from some older code I think.
After looking at this further this variable wasn't even used so
I will trim this in upcoming v3 patch.
Regards,
Martin
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
RE: [PATCH v2] DA8XX/OMAP-L1XX: FB: Implement double buffering
On Thu, Apr 01, 2010 at 01:47:40, Jon Povey wrote: Andrew Morton wrote: On Wed, 31 Mar 2010 20:43:29 -0500 Ambrose, Martin mar...@ti.com wrote: If the calling process has signal_pending() (say, someone hit ^C) then wait_event_interruptible_timeout() will fall straight through with -ERESTARTSYS. Will this cause the driver to malfunction at all? I don't think so since the driver doesn't make use of this information in any way. This is just status to the caller that the current frame has finished DMA'ing out of the framebuffer. Could you maybe propose a scenario/use case where you think it is problematic? I could then either reason why it should be OK or I'll create a test harness and see how the driver can/should be modified. Gee, I dunno - I don't understand the driver to that level. If you're OK with this wait being interrupted by a signal and the driver handles it OK then fine, that's a feature. To test it I suppose you should give your test app a signal handler and blast kills at it from another process. Jumping in.. This should not cause a problem for the driver; its purpose is to tell userland that it can write to the buffer without corrupting graphics (as presumably it is double-buffering and the other buffer is now being DMA'd from by the hardware). At worst, if the userland software doesn't handle this correctly it may draw one bad frame of graphics. Although if it's had a ctrl-C it probably has bigger things to worry about. If the app wants to handle signals it needs to consider such things.. I would not expect the driver to do anything other than what this patch does. Thanks Jon. I agree. If no further objections I'll leave this as is. Regards, Martin ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
RE: [PATCH v2] DA8XX/OMAP-L1XX: FB: Implement double buffering
Andrew Morton wrote: On Wed, 31 Mar 2010 20:43:29 -0500 Ambrose, Martin mar...@ti.com wrote: If the calling process has signal_pending() (say, someone hit ^C) then wait_event_interruptible_timeout() will fall straight through with -ERESTARTSYS. Will this cause the driver to malfunction at all? I don't think so since the driver doesn't make use of this information in any way. This is just status to the caller that the current frame has finished DMA'ing out of the framebuffer. Could you maybe propose a scenario/use case where you think it is problematic? I could then either reason why it should be OK or I'll create a test harness and see how the driver can/should be modified. Gee, I dunno - I don't understand the driver to that level. If you're OK with this wait being interrupted by a signal and the driver handles it OK then fine, that's a feature. To test it I suppose you should give your test app a signal handler and blast kills at it from another process. Jumping in.. This should not cause a problem for the driver; its purpose is to tell userland that it can write to the buffer without corrupting graphics (as presumably it is double-buffering and the other buffer is now being DMA'd from by the hardware). At worst, if the userland software doesn't handle this correctly it may draw one bad frame of graphics. Although if it's had a ctrl-C it probably has bigger things to worry about. If the app wants to handle signals it needs to consider such things.. I would not expect the driver to do anything other than what this patch does. -- Jon Povey jon.po...@racelogic.co.uk Racelogic is a limited company registered in England. Registered number 2743719 . Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB . The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
RE: [PATCH v2] DA8XX/OMAP-L1XX: FB: Implement double buffering
Andrew,
Thanks for the feedback.
+/*
+ * Function to wait for vertical sync which for this LCD peripheral
+ * translates into waiting for the current raster frame to complete.
+ */
+static int fb_wait_for_vsync(struct fb_info *info)
+{
+ struct da8xx_fb_par *par = info-par;
+ wait_queue_t wq;
+ int ret;
+
+ init_waitqueue_entry(wq, current);
DECLARE_WAITQUEUE() would be more conventional.
I'll update -- cloned this from some older code I think.
+ /*
+* Set flag to 0 and wait for isr to set to 1. It would seem there
is a
+* race condition here where the ISR could have occured just before
or
+* just after this set. But since we are just coarsely waiting for
+* a frame to complete then that's OK. i.e. if the frame completed
+* just before this code executed then we have to wait another full
+* frame time but there is no way to avoid such a situation. On the
+* other hand if the frame completed just after then we don't need
+* to wait long at all. Either way we are guaranteed to return to
the
+* user immediately after a frame completion which is all that is
+* required.
+*/
+ par-vsync_flag = 0;
+ ret = wait_event_interruptible_timeout(par-vsync_wait,
+ par-vsync_flag != 0,
+ par-vsync_timeout);
If the calling process has signal_pending() (say, someone hit ^C) then
wait_event_interruptible_timeout() will fall straight through with
-ERESTARTSYS. Will this cause the driver to malfunction at all?
I don't think so since the driver doesn't make use of this
information in any way. This is just status to the caller that the
current frame has finished DMA'ing out of the framebuffer.
Could you maybe propose a scenario/use case where you think it is
problematic? I could then either reason why it should be OK or
I'll create a test harness and see how the driver can/should be modified.
Regards,
Martin
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2] DA8XX/OMAP-L1XX: FB: Implement double buffering
On Wed, 31 Mar 2010 20:43:29 -0500 Ambrose, Martin mar...@ti.com wrote: + /* +* Set flag to 0 and wait for isr to set to 1. It would seem there is a +* race condition here where the ISR could have occured just before or +* just after this set. But since we are just coarsely waiting for +* a frame to complete then that's OK. i.e. if the frame completed +* just before this code executed then we have to wait another full +* frame time but there is no way to avoid such a situation. On the +* other hand if the frame completed just after then we don't need +* to wait long at all. Either way we are guaranteed to return to the +* user immediately after a frame completion which is all that is +* required. +*/ + par-vsync_flag = 0; + ret = wait_event_interruptible_timeout(par-vsync_wait, + par-vsync_flag != 0, + par-vsync_timeout); If the calling process has signal_pending() (say, someone hit ^C) then wait_event_interruptible_timeout() will fall straight through with -ERESTARTSYS. Will this cause the driver to malfunction at all? I don't think so since the driver doesn't make use of this information in any way. This is just status to the caller that the current frame has finished DMA'ing out of the framebuffer. Could you maybe propose a scenario/use case where you think it is problematic? I could then either reason why it should be OK or I'll create a test harness and see how the driver can/should be modified. Gee, I dunno - I don't understand the driver to that level. If you're OK with this wait being interrupted by a signal and the driver handles it OK then fine, that's a feature. To test it I suppose you should give your test app a signal handler and blast kills at it from another process. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2] DA8XX/OMAP-L1XX: FB: Implement double buffering
On Mon, 29 Mar 2010 08:16:51 -0500
Ambrose, Martin mar...@ti.com wrote:
+/*
+ * Function to wait for vertical sync which for this LCD peripheral
+ * translates into waiting for the current raster frame to complete.
+ */
+static int fb_wait_for_vsync(struct fb_info *info)
+{
+ struct da8xx_fb_par *par = info-par;
+ wait_queue_t wq;
+ int ret;
+
+ init_waitqueue_entry(wq, current);
DECLARE_WAITQUEUE() would be more conventional.
+ /*
+* Set flag to 0 and wait for isr to set to 1. It would seem there is
a
+* race condition here where the ISR could have occured just before or
+* just after this set. But since we are just coarsely waiting for
+* a frame to complete then that's OK. i.e. if the frame completed
+* just before this code executed then we have to wait another full
+* frame time but there is no way to avoid such a situation. On the
+* other hand if the frame completed just after then we don't need
+* to wait long at all. Either way we are guaranteed to return to the
+* user immediately after a frame completion which is all that is
+* required.
+*/
+ par-vsync_flag = 0;
+ ret = wait_event_interruptible_timeout(par-vsync_wait,
+ par-vsync_flag != 0,
+ par-vsync_timeout);
If the calling process has signal_pending() (say, someone hit ^C) then
wait_event_interruptible_timeout() will fall straight through with
-ERESTARTSYS. Will this cause the driver to malfunction at all?
+ if (ret 0)
+ return ret;
+ if (ret == 0)
+ return -ETIMEDOUT;
+
+ return 0;
+}
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
