| Message ID | 20220718214123.73275-3-ezequiel@vanguardiasur.com.ar (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hantro: Remove incorrect HEVC SPS validation | expand |
On Mon, Jul 18, 2022 at 06:41:23PM -0300, Ezequiel Garcia wrote: > Currently, the driver tries to validat the HEVC SPS > against the CAPTURE queue format (i.e. the decoded format). > This is not correct, because typically the SPS control is set > before the CAPTURE queue is negotiated. > > Fixes: 135ad96cb4d6b ("media: hantro: Be more accurate on pixel formats step_width constraints") > Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> This is actually a v2. I've chosen to move the control checks back to try_ctrl, but leave the tiled format alignment check. This ALIGN() check can be optimized, moving the alignment to the TRY_FMT step (instead of failing at device_run). However, this is a minor optimization, so I'd like to avoid the scope creep, and pospone the changes regarding the tiled format resolution alignment. Thanks, Ezequiel > --- > drivers/media/platform/verisilicon/hantro_drv.c | 12 ++++++------ > drivers/media/platform/verisilicon/hantro_hevc.c | 9 +-------- > drivers/media/platform/verisilicon/hantro_hw.h | 1 - > 3 files changed, 7 insertions(+), 15 deletions(-) > > diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c > index e5fc0a99b728..2036f72eeb4a 100644 > --- a/drivers/media/platform/verisilicon/hantro_drv.c > +++ b/drivers/media/platform/verisilicon/hantro_drv.c > @@ -251,11 +251,6 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq) > > static int hantro_try_ctrl(struct v4l2_ctrl *ctrl) > { > - struct hantro_ctx *ctx; > - > - ctx = container_of(ctrl->handler, > - struct hantro_ctx, ctrl_handler); > - > if (ctrl->id == V4L2_CID_STATELESS_H264_SPS) { > const struct v4l2_ctrl_h264_sps *sps = ctrl->p_new.p_h264_sps; > > @@ -271,7 +266,12 @@ static int hantro_try_ctrl(struct v4l2_ctrl *ctrl) > } else if (ctrl->id == V4L2_CID_STATELESS_HEVC_SPS) { > const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps; > > - return hantro_hevc_validate_sps(ctx, sps); > + if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8) > + /* Luma and chroma bit depth mismatch */ > + return -EINVAL; > + if (sps->bit_depth_luma_minus8 != 0) > + /* Only 8-bit is supported */ > + return -EINVAL; > } else if (ctrl->id == V4L2_CID_STATELESS_VP9_FRAME) { > const struct v4l2_ctrl_vp9_frame *dec_params = ctrl->p_new.p_vp9_frame; > > diff --git a/drivers/media/platform/verisilicon/hantro_hevc.c b/drivers/media/platform/verisilicon/hantro_hevc.c > index 5984c5fa6f83..b990bc98164c 100644 > --- a/drivers/media/platform/verisilicon/hantro_hevc.c > +++ b/drivers/media/platform/verisilicon/hantro_hevc.c > @@ -154,15 +154,8 @@ static int tile_buffer_reallocate(struct hantro_ctx *ctx) > return -ENOMEM; > } > > -int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps) > +static int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps) > { > - if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8) > - /* Luma and chroma bit depth mismatch */ > - return -EINVAL; > - if (sps->bit_depth_luma_minus8 != 0) > - /* Only 8-bit is supported */ > - return -EINVAL; > - > /* > * for tile pixel format check if the width and height match > * hardware constraints > diff --git a/drivers/media/platform/verisilicon/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h > index 762631d15fdc..e83f0c523a30 100644 > --- a/drivers/media/platform/verisilicon/hantro_hw.h > +++ b/drivers/media/platform/verisilicon/hantro_hw.h > @@ -360,7 +360,6 @@ int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx); > void hantro_hevc_ref_init(struct hantro_ctx *ctx); > dma_addr_t hantro_hevc_get_ref_buf(struct hantro_ctx *ctx, s32 poc); > int hantro_hevc_add_ref_buf(struct hantro_ctx *ctx, int poc, dma_addr_t addr); > -int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps); > > > static inline unsigned short hantro_vp9_num_sbs(unsigned short dimension) > -- > 2.34.3 >
diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c index e5fc0a99b728..2036f72eeb4a 100644 --- a/drivers/media/platform/verisilicon/hantro_drv.c +++ b/drivers/media/platform/verisilicon/hantro_drv.c @@ -251,11 +251,6 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq) static int hantro_try_ctrl(struct v4l2_ctrl *ctrl) { - struct hantro_ctx *ctx; - - ctx = container_of(ctrl->handler, - struct hantro_ctx, ctrl_handler); - if (ctrl->id == V4L2_CID_STATELESS_H264_SPS) { const struct v4l2_ctrl_h264_sps *sps = ctrl->p_new.p_h264_sps; @@ -271,7 +266,12 @@ static int hantro_try_ctrl(struct v4l2_ctrl *ctrl) } else if (ctrl->id == V4L2_CID_STATELESS_HEVC_SPS) { const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps; - return hantro_hevc_validate_sps(ctx, sps); + if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8) + /* Luma and chroma bit depth mismatch */ + return -EINVAL; + if (sps->bit_depth_luma_minus8 != 0) + /* Only 8-bit is supported */ + return -EINVAL; } else if (ctrl->id == V4L2_CID_STATELESS_VP9_FRAME) { const struct v4l2_ctrl_vp9_frame *dec_params = ctrl->p_new.p_vp9_frame; diff --git a/drivers/media/platform/verisilicon/hantro_hevc.c b/drivers/media/platform/verisilicon/hantro_hevc.c index 5984c5fa6f83..b990bc98164c 100644 --- a/drivers/media/platform/verisilicon/hantro_hevc.c +++ b/drivers/media/platform/verisilicon/hantro_hevc.c @@ -154,15 +154,8 @@ static int tile_buffer_reallocate(struct hantro_ctx *ctx) return -ENOMEM; } -int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps) +static int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps) { - if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8) - /* Luma and chroma bit depth mismatch */ - return -EINVAL; - if (sps->bit_depth_luma_minus8 != 0) - /* Only 8-bit is supported */ - return -EINVAL; - /* * for tile pixel format check if the width and height match * hardware constraints diff --git a/drivers/media/platform/verisilicon/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h index 762631d15fdc..e83f0c523a30 100644 --- a/drivers/media/platform/verisilicon/hantro_hw.h +++ b/drivers/media/platform/verisilicon/hantro_hw.h @@ -360,7 +360,6 @@ int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx); void hantro_hevc_ref_init(struct hantro_ctx *ctx); dma_addr_t hantro_hevc_get_ref_buf(struct hantro_ctx *ctx, s32 poc); int hantro_hevc_add_ref_buf(struct hantro_ctx *ctx, int poc, dma_addr_t addr); -int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps); static inline unsigned short hantro_vp9_num_sbs(unsigned short dimension)
Currently, the driver tries to validat the HEVC SPS against the CAPTURE queue format (i.e. the decoded format). This is not correct, because typically the SPS control is set before the CAPTURE queue is negotiated. Fixes: 135ad96cb4d6b ("media: hantro: Be more accurate on pixel formats step_width constraints") Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> --- drivers/media/platform/verisilicon/hantro_drv.c | 12 ++++++------ drivers/media/platform/verisilicon/hantro_hevc.c | 9 +-------- drivers/media/platform/verisilicon/hantro_hw.h | 1 - 3 files changed, 7 insertions(+), 15 deletions(-)