| Message ID | 20200724072218.943245-4-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush, On 24/07/2020 08:22, Naushir Patuck wrote: > Add a debug log point to indicate a request has completed. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/libcamera/request.cpp | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > index 6b9e0b4a..cba1715e 100644 > --- a/src/libcamera/request.cpp > +++ b/src/libcamera/request.cpp > @@ -212,6 +212,9 @@ void Request::complete() > { > ASSERT(!hasPendingBuffers()); > status_ = cancelled_ ? RequestCancelled : RequestComplete; > + > + LOG(Request, Debug) << "Request has completed - cookie: " > + << cookie_; sorry to extend this with feature creep, but if we're reporting a completion here, and we've just calculated the status_, should we print the result? Or perhaps, only print [Cancelled] if it was a cancelled ... Would this make sense, or be helpful? (untested) << cancelled_ ? " [Cancelled]" : ""; We could swap out the 'has completed' for 'has been cancelled', but I would think it might be useful to be easy to spot cancelled requests. > } > > /** >
Hi Kieran, On Fri, 24 Jul 2020 at 10:46, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Hi Naush, > > On 24/07/2020 08:22, Naushir Patuck wrote: > > Add a debug log point to indicate a request has completed. > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > --- > > src/libcamera/request.cpp | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > > index 6b9e0b4a..cba1715e 100644 > > --- a/src/libcamera/request.cpp > > +++ b/src/libcamera/request.cpp > > @@ -212,6 +212,9 @@ void Request::complete() > > { > > ASSERT(!hasPendingBuffers()); > > status_ = cancelled_ ? RequestCancelled : RequestComplete; > > + > > + LOG(Request, Debug) << "Request has completed - cookie: " > > + << cookie_; > > sorry to extend this with feature creep, but if we're reporting a > completion here, and we've just calculated the status_, should we print > the result? > > Or perhaps, only print [Cancelled] if it was a cancelled ... > > Would this make sense, or be helpful? (untested) > > << cancelled_ ? " [Cancelled]" : ""; > > > We could swap out the 'has completed' for 'has been cancelled', but I > would think it might be useful to be easy to spot cancelled requests. > Sure, that seems like a useful thing to do. New patchset incoming... Regards, Naush > > } > > > > /** > > > > -- > Regards > -- > Kieran
Hi Naush, On 24/07/2020 11:23, Naushir Patuck wrote: > Hi Kieran, > > On Fri, 24 Jul 2020 at 10:46, Kieran Bingham > <kieran.bingham@ideasonboard.com> wrote: >> >> Hi Naush, >> >> On 24/07/2020 08:22, Naushir Patuck wrote: >>> Add a debug log point to indicate a request has completed. >>> >>> Signed-off-by: Naushir Patuck <naush@raspberrypi.com> >>> --- >>> src/libcamera/request.cpp | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp >>> index 6b9e0b4a..cba1715e 100644 >>> --- a/src/libcamera/request.cpp >>> +++ b/src/libcamera/request.cpp >>> @@ -212,6 +212,9 @@ void Request::complete() >>> { >>> ASSERT(!hasPendingBuffers()); >>> status_ = cancelled_ ? RequestCancelled : RequestComplete; >>> + >>> + LOG(Request, Debug) << "Request has completed - cookie: " >>> + << cookie_; >> >> sorry to extend this with feature creep, but if we're reporting a >> completion here, and we've just calculated the status_, should we print >> the result? >> >> Or perhaps, only print [Cancelled] if it was a cancelled ... >> >> Would this make sense, or be helpful? (untested) >> >> << cancelled_ ? " [Cancelled]" : ""; >> >> >> We could swap out the 'has completed' for 'has been cancelled', but I >> would think it might be useful to be easy to spot cancelled requests. >> > > Sure, that seems like a useful thing to do. New patchset incoming... No need to resend this whole series yet for this, you can just repost this one patch ... Mark it as v5.1 or something ;-) > > Regards, > Naush > > >>> } >>> >>> /** >>> >> >> -- >> Regards >> -- >> Kieran
On 24/07/2020 11:28, Kieran Bingham wrote: > Hi Naush, > > On 24/07/2020 11:23, Naushir Patuck wrote: >> Hi Kieran, >> >> On Fri, 24 Jul 2020 at 10:46, Kieran Bingham >> <kieran.bingham@ideasonboard.com> wrote: >>> >>> Hi Naush, >>> >>> On 24/07/2020 08:22, Naushir Patuck wrote: >>>> Add a debug log point to indicate a request has completed. >>>> >>>> Signed-off-by: Naushir Patuck <naush@raspberrypi.com> >>>> --- >>>> src/libcamera/request.cpp | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp >>>> index 6b9e0b4a..cba1715e 100644 >>>> --- a/src/libcamera/request.cpp >>>> +++ b/src/libcamera/request.cpp >>>> @@ -212,6 +212,9 @@ void Request::complete() >>>> { >>>> ASSERT(!hasPendingBuffers()); >>>> status_ = cancelled_ ? RequestCancelled : RequestComplete; >>>> + >>>> + LOG(Request, Debug) << "Request has completed - cookie: " >>>> + << cookie_; >>> >>> sorry to extend this with feature creep, but if we're reporting a >>> completion here, and we've just calculated the status_, should we print >>> the result? >>> >>> Or perhaps, only print [Cancelled] if it was a cancelled ... >>> >>> Would this make sense, or be helpful? (untested) >>> >>> << cancelled_ ? " [Cancelled]" : ""; >>> >>> >>> We could swap out the 'has completed' for 'has been cancelled', but I >>> would think it might be useful to be easy to spot cancelled requests. >>> >> >> Sure, that seems like a useful thing to do. New patchset incoming... > > > No need to resend this whole series yet for this, you can just repost > this one patch ... > > Mark it as v5.1 or something ;-) Oh - and you can add --in-reply-to="<20200724072218.943245-4-naush@raspberrypi.com>" to git-send-email to make it follow your v5 ;-) > >> >> Regards, >> Naush >> >> >>>> } >>>> >>>> /** >>>> >>> >>> -- >>> Regards >>> -- >>> Kieran >
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 6b9e0b4a..cba1715e 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -212,6 +212,9 @@ void Request::complete() { ASSERT(!hasPendingBuffers()); status_ = cancelled_ ? RequestCancelled : RequestComplete; + + LOG(Request, Debug) << "Request has completed - cookie: " + << cookie_; } /**
Add a debug log point to indicate a request has completed. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- src/libcamera/request.cpp | 3 +++ 1 file changed, 3 insertions(+)