On 09/15/2016 12:30 PM, Hans Verkuil wrote:
> Same thing here: this should be enabled in a Kconfig option. That new Kconfig
> option should depend on DEBUG_FS as well.
>
> BTW (independent of these two patches): I think it would be easier to maintain
> if you make a Kconfig file in the sti directory, rather than adding them to
> the
> Kconfig in platform.
>
> Regards,
>
> Hans
Hi Hans,
Thank you for your remarks about the 2 debug patches for HVA.
I've just sent a version 2 with the following propositions:
- the encoding summary (first patch) doesn't include any longer information
about the encoding performance. There's no more computation in this patch
(hva_dbg_perf_begin and hva_dbg_perf_end removed). Thus, there's no potential
penalty on the encoding performance. So, I propose that the short summary (very
useful to easily get a status about the encoding operation) is always enabled
(dev_info).
- for the second patch, I propose that the computation of the performances
(hva_dbg_perf_begin and hva_dbg_perf_end) is conditioned by CONFIG_DEBUG_FS. It
avoids the definition of a new Kconfig option (mkt-vpu also uses
CONFIG_DEBUG_FS). As to the functions that manage the debugfs entries
(hva_*_create/remove), I propose that they are called even if DEBUG_FS is
disabled: in this case, thanks to the debugfs API, the entries will simply not
created.
Finally, I think that your proposition of having a Kconfig in the sti directory
makes sense. It might be part of a different patch series dealing with the "sti
coherency" (we discussed about that on IRC with Benjamin Gaignard few weeks
ago).
Regards,
Jean-Christophe.
>
> On 09/12/2016 06:01 PM, Jean-Christophe Trotin wrote:
>> This patch creates 4 static debugfs entries to dump:
>> - the device-related information ("st-hva/device")
>> - the list of registered encoders ("st-hva/encoders")
>> - the current values of the hva registers ("st-hva/regs")
>> - the information about the last closed instance ("st-hva/last")
>>
>> It also creates dynamically a debugfs entry for each opened instance,
>> ("st-hva/") to dump:
>> - the information about the stream (profile, level, resolution,
>> alignment...)
>> - the control parameters (bitrate mode, framerate, GOP size...)
>> - the potential (system, encoding...) errors
>> - the performance information about the encoding (HW processing
>> duration, average bitrate, average framerate...)
>> Each time a running instance is closed, its context (including the
>> debug information) is saved to feed, on demand, the last closed
>> instance debugfs entry.
>>
>> Signed-off-by: Yannick Fertre
>> Signed-off-by: Jean-Christophe Trotin
>> ---
>> drivers/media/platform/sti/hva/hva-debug.c | 362
>> +
>> drivers/media/platform/sti/hva/hva-hw.c| 39
>> drivers/media/platform/sti/hva/hva-hw.h| 1 +
>> drivers/media/platform/sti/hva/hva-v4l2.c | 11 +-
>> drivers/media/platform/sti/hva/hva.h | 86 +--
>> 5 files changed, 482 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/media/platform/sti/hva/hva-debug.c
>> b/drivers/media/platform/sti/hva/hva-debug.c
>> index 71bbf32..433b1d4 100644
>> --- a/drivers/media/platform/sti/hva/hva-debug.c
>> +++ b/drivers/media/platform/sti/hva/hva-debug.c
>> @@ -5,7 +5,113 @@
>> * License terms: GNU General Public License (GPL), version 2
>> */
>>
>> +#include
>> +
>> #include "hva.h"
>> +#include "hva-hw.h"
>> +
>> +static void format_ctx(struct seq_file *s, struct hva_ctx *ctx)
>> +{
>> + struct hva_streaminfo *stream = &ctx->streaminfo;
>> + struct hva_frameinfo *frame = &ctx->frameinfo;
>> + struct hva_controls *ctrls = &ctx->ctrls;
>> + struct hva_ctx_dbg *dbg = &ctx->dbg;
>> + u32 bitrate_mode, aspect, entropy, vui_sar, sei_fp;
>> +
>> + seq_printf(s, "|-%s\n |\n", ctx->name);
>> +
>> + seq_printf(s, " |-[%sframe info]\n",
>> +ctx->flags & HVA_FLAG_FRAMEINFO ? "" : "default ");
>> + seq_printf(s, " | |- pixel format=%4.4s\n"
>> + " | |- wxh=%dx%d\n"
>> + " | |- wxh (w/ encoder alignment constraint)=%dx%d\n"
>> + " |\n",
>> + (char *)&frame->pixelformat,
>> + frame->width, frame->height,
>> + frame->aligned_width, frame->aligned_height);
>> +
>> + seq_printf(s, " |-[%sstream info]\n",
>> +ctx->flags & HVA_FLAG_STREAMINFO ? "" : "default ");
>> + seq_printf(s, " | |- stream format=%4.4s\n"
>> + " | |- wxh=%dx%d\n"
>> + " | |- %s\n"
>> + " | |- %s\n"
>> + " |\n",
>> + (char *)&stream->streamformat,
>> + stream->width, stream->height,
>> + stream->profile, stream->level);
>> +
>> + bitrate_mode = V4L2_CID_MPEG_VIDEO_BITRATE_MODE;
>> + aspect = V4L2_CID_MPEG_VIDEO_ASPECT;
>> + seq_puts(s, " |-[
