lkml.org 
[lkml]   [2018]   [Sep]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v9 5/9] media: platform: Add Cedrus VPU decoder driver
    Em Fri,  7 Sep 2018 00:24:38 +0200
    Paul Kocialkowski <contact@paulk.fr> escreveu:

    > From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
    >
    > This introduces the Cedrus VPU driver that supports the VPU found in
    > Allwinner SoCs, also known as Video Engine. It is implemented through
    > a V4L2 M2M decoder device and a media device (used for media requests).
    > So far, it only supports MPEG-2 decoding.
    >
    > Since this VPU is stateless, synchronization with media requests is
    > required in order to ensure consistency between frame headers that
    > contain metadata about the frame to process and the raw slice data that
    > is used to generate the frame.
    >
    > This driver was made possible thanks to the long-standing effort
    > carried out by the linux-sunxi community in the interest of reverse
    > engineering, documenting and implementing support for the Allwinner VPU.
    >
    > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
    > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>

    There are several checkpatch issues here. Ok, some can be
    ignored, but there are at least some of them that sounda relevant.

    Please address them.


    CHECK: Comparison to NULL could be written "ctx->ctrls[i]"
    #214: FILE: drivers/staging/media/sunxi/cedrus/cedrus.c:51:
    + for (i = 0; ctx->ctrls[i] != NULL; i++)

    CHECK: Alignment should match open parenthesis
    #304: FILE: drivers/staging/media/sunxi/cedrus/cedrus.c:141:
    + ctrl_test = v4l2_ctrl_request_hdl_ctrl_find(hdl,
    + cedrus_controls[i].id);

    CHECK: Alignment should match open parenthesis
    #468: FILE: drivers/staging/media/sunxi/cedrus/cedrus.c:305:
    + ret = v4l2_m2m_register_media_controller(dev->m2m_dev,
    + vfd, MEDIA_ENT_F_PROC_VIDEO_DECODER);

    CHECK: Avoid using bool structure members because of possible alignment issues - see: https://lkml.org/lkml/2017/11/21/384
    #638: FILE: drivers/staging/media/sunxi/cedrus/cedrus.h:47:
    + bool required;

    WARNING: line over 80 characters
    #744: FILE: drivers/staging/media/sunxi/cedrus/cedrus.h:153:
    +static inline struct cedrus_buffer *vb2_v4l2_to_cedrus_buffer(const struct vb2_v4l2_buffer *p)

    WARNING: line over 80 characters
    #749: FILE: drivers/staging/media/sunxi/cedrus/cedrus.h:158:
    +static inline struct cedrus_buffer *vb2_to_cedrus_buffer(const struct vb2_buffer *p)

    CHECK: Alignment should match open parenthesis
    #809: FILE: drivers/staging/media/sunxi/cedrus/cedrus_dec.c:47:
    + run.mpeg2.slice_params = cedrus_find_control_data(ctx,
    + V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS);

    CHECK: Alignment should match open parenthesis
    #811: FILE: drivers/staging/media/sunxi/cedrus/cedrus_dec.c:49:
    + run.mpeg2.quantization = cedrus_find_control_data(ctx,
    + V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION);

    WARNING: line over 80 characters
    #1368: FILE: drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c:133:
    + reg |= VE_DEC_MPEG_MP12HDR_INTRA_DC_PRECISION(picture->intra_dc_precision);

    WARNING: line over 80 characters
    #1369: FILE: drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c:134:
    + reg |= VE_DEC_MPEG_MP12HDR_INTRA_PICTURE_STRUCTURE(picture->picture_structure);

    WARNING: line over 80 characters
    #1371: FILE: drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c:136:
    + reg |= VE_DEC_MPEG_MP12HDR_FRAME_PRED_FRAME_DCT(picture->frame_pred_frame_dct);

    WARNING: line over 80 characters
    #1372: FILE: drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c:137:
    + reg |= VE_DEC_MPEG_MP12HDR_CONCEALMENT_MOTION_VECTORS(picture->concealment_motion_vectors);

    WARNING: line over 80 characters
    #1395: FILE: drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c:160:
    + fwd_luma_addr = cedrus_dst_buf_addr(ctx, slice_params->forward_ref_index, 0);

    WARNING: line over 80 characters
    #1396: FILE: drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c:161:
    + fwd_chroma_addr = cedrus_dst_buf_addr(ctx, slice_params->forward_ref_index, 1);

    WARNING: line over 80 characters
    #1401: FILE: drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c:166:
    + bwd_luma_addr = cedrus_dst_buf_addr(ctx, slice_params->backward_ref_index, 0);

    WARNING: line over 80 characters
    #1402: FILE: drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c:167:
    + bwd_chroma_addr = cedrus_dst_buf_addr(ctx, slice_params->backward_ref_index, 1);

    WARNING: line over 80 characters
    #1417: FILE: drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c:182:
    + cedrus_write(dev, VE_DEC_MPEG_VLD_OFFSET, slice_params->data_bit_offset);

    CHECK: Macro argument reuse 'x' - possible side-effects?
    #1552: FILE: drivers/staging/media/sunxi/cedrus/cedrus_regs.h:74:
    +#define VE_DEC_MPEG_MP12HDR_F_CODE_MASK(x, y) \
    + GENMASK(VE_DEC_MPEG_MP12HDR_F_CODE_SHIFT(x, y) + 3, \
    + VE_DEC_MPEG_MP12HDR_F_CODE_SHIFT(x, y))

    CHECK: Macro argument reuse 'y' - possible side-effects?
    #1552: FILE: drivers/staging/media/sunxi/cedrus/cedrus_regs.h:74:
    +#define VE_DEC_MPEG_MP12HDR_F_CODE_MASK(x, y) \
    + GENMASK(VE_DEC_MPEG_MP12HDR_F_CODE_SHIFT(x, y) + 3, \
    + VE_DEC_MPEG_MP12HDR_F_CODE_SHIFT(x, y))

    CHECK: Macro argument reuse 'x' - possible side-effects?
    #1555: FILE: drivers/staging/media/sunxi/cedrus/cedrus_regs.h:77:
    +#define VE_DEC_MPEG_MP12HDR_F_CODE(x, y, v) \
    + (((v) << VE_DEC_MPEG_MP12HDR_F_CODE_SHIFT(x, y)) & \
    + VE_DEC_MPEG_MP12HDR_F_CODE_MASK(x, y))

    CHECK: Macro argument reuse 'y' - possible side-effects?
    #1555: FILE: drivers/staging/media/sunxi/cedrus/cedrus_regs.h:77:
    +#define VE_DEC_MPEG_MP12HDR_F_CODE(x, y, v) \
    + (((v) << VE_DEC_MPEG_MP12HDR_F_CODE_SHIFT(x, y)) & \
    + VE_DEC_MPEG_MP12HDR_F_CODE_MASK(x, y))

    CHECK: Macro argument reuse 'a' - possible side-effects?
    #1685: FILE: drivers/staging/media/sunxi/cedrus/cedrus_regs.h:207:
    +#define VE_DEC_MPEG_VLD_ADDR_BASE(a) \
    + (((a) & GENMASK(27, 4)) | (((a) >> 28) & GENMASK(3, 0)))

    CHECK: Comparison to NULL could be written "fmt"
    #1805: FILE: drivers/staging/media/sunxi/cedrus/cedrus_video.c:88:
    + return fmt != NULL;

    CHECK: Please use a blank line after function/struct/union/enum declarations
    #2214: FILE: drivers/staging/media/sunxi/cedrus/cedrus_video.c:497:
    +}
    +static struct vb2_ops cedrus_qops = {

    total: 0 errors, 11 warnings, 13 checks, 2139 lines checked






    Thanks,
    Mauro

    \
     
     \ /
      Last update: 2018-09-11 17:47    [W:3.720 / U:0.364 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site