| Message ID | HE1PR06MB4011544CF7A6F36EF1CA47EEAC610@HE1PR06MB4011.eurprd06.prod.outlook.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: hantro: H264 fixes and improvements | expand |
On Tue, 29 Oct 2019 01:24:50 +0000 Jonas Karlman <jonas@kwiboo.se> wrote: > Calculations for motion vector buffer offset is based on width and height > from the configured capture format, lets use the same values for macroblock > width and height hw regs. > > Fixes: dea0a82f3d22 ("media: hantro: Add support for H264 decoding on G1") > Signed-off-by: Jonas Karlman <jonas@kwiboo.se> > --- > Changes in v2: > - new patch split from "media: hantro: Fix H264 motion vector buffer offset" > --- > drivers/staging/media/hantro/hantro_g1_h264_dec.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/staging/media/hantro/hantro_g1_h264_dec.c > index 71bf162eaf73..eeed11366135 100644 > --- a/drivers/staging/media/hantro/hantro_g1_h264_dec.c > +++ b/drivers/staging/media/hantro/hantro_g1_h264_dec.c > @@ -51,8 +51,8 @@ static void set_params(struct hantro_ctx *ctx) > vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL0); > > /* Decoder control register 1. */ > - reg = G1_REG_DEC_CTRL1_PIC_MB_WIDTH(sps->pic_width_in_mbs_minus1 + 1) | > - G1_REG_DEC_CTRL1_PIC_MB_HEIGHT_P(sps->pic_height_in_map_units_minus1 + 1) | > + reg = G1_REG_DEC_CTRL1_PIC_MB_WIDTH(MB_WIDTH(ctx->dst_fmt.width)) | > + G1_REG_DEC_CTRL1_PIC_MB_HEIGHT_P(MB_HEIGHT(ctx->dst_fmt.height)) | I'm just curious, do they differ in practice? Also not sure what's been decided for the "G1 post-proc", but if the dst/capture format res set by the user is the scaled res (PP can scale IIRC), then you probably shouldn't use those values here. > G1_REG_DEC_CTRL1_REF_FRAMES(sps->max_num_ref_frames); > vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL1); >
On 2019-10-31 10:21, Boris Brezillon wrote: > On Tue, 29 Oct 2019 01:24:50 +0000 > Jonas Karlman <jonas@kwiboo.se> wrote: > >> Calculations for motion vector buffer offset is based on width and height >> from the configured capture format, lets use the same values for macroblock >> width and height hw regs. >> >> Fixes: dea0a82f3d22 ("media: hantro: Add support for H264 decoding on G1") >> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> >> --- >> Changes in v2: >> - new patch split from "media: hantro: Fix H264 motion vector buffer offset" >> --- >> drivers/staging/media/hantro/hantro_g1_h264_dec.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/staging/media/hantro/hantro_g1_h264_dec.c >> index 71bf162eaf73..eeed11366135 100644 >> --- a/drivers/staging/media/hantro/hantro_g1_h264_dec.c >> +++ b/drivers/staging/media/hantro/hantro_g1_h264_dec.c >> @@ -51,8 +51,8 @@ static void set_params(struct hantro_ctx *ctx) >> vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL0); >> >> /* Decoder control register 1. */ >> - reg = G1_REG_DEC_CTRL1_PIC_MB_WIDTH(sps->pic_width_in_mbs_minus1 + 1) | >> - G1_REG_DEC_CTRL1_PIC_MB_HEIGHT_P(sps->pic_height_in_map_units_minus1 + 1) | >> + reg = G1_REG_DEC_CTRL1_PIC_MB_WIDTH(MB_WIDTH(ctx->dst_fmt.width)) | >> + G1_REG_DEC_CTRL1_PIC_MB_HEIGHT_P(MB_HEIGHT(ctx->dst_fmt.height)) | > I'm just curious, do they differ in practice? Also not sure what's been > decided for the "G1 post-proc", but if the dst/capture format res set > by the user is the scaled res (PP can scale IIRC), then you probably > shouldn't use those values here. You are correct, I wanted to use the same source for both size and offsets, looking at this again both here and where is it used for offsets this probably need to change. Do you think we can use src_fmt.width/height for size and offsets? It looks like that is what cedrus is using. Regards, Jonas > >> G1_REG_DEC_CTRL1_REF_FRAMES(sps->max_num_ref_frames); >> vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL1); >> >>
diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/staging/media/hantro/hantro_g1_h264_dec.c index 71bf162eaf73..eeed11366135 100644 --- a/drivers/staging/media/hantro/hantro_g1_h264_dec.c +++ b/drivers/staging/media/hantro/hantro_g1_h264_dec.c @@ -51,8 +51,8 @@ static void set_params(struct hantro_ctx *ctx) vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL0); /* Decoder control register 1. */ - reg = G1_REG_DEC_CTRL1_PIC_MB_WIDTH(sps->pic_width_in_mbs_minus1 + 1) | - G1_REG_DEC_CTRL1_PIC_MB_HEIGHT_P(sps->pic_height_in_map_units_minus1 + 1) | + reg = G1_REG_DEC_CTRL1_PIC_MB_WIDTH(MB_WIDTH(ctx->dst_fmt.width)) | + G1_REG_DEC_CTRL1_PIC_MB_HEIGHT_P(MB_HEIGHT(ctx->dst_fmt.height)) | G1_REG_DEC_CTRL1_REF_FRAMES(sps->max_num_ref_frames); vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL1);
Calculations for motion vector buffer offset is based on width and height from the configured capture format, lets use the same values for macroblock width and height hw regs. Fixes: dea0a82f3d22 ("media: hantro: Add support for H264 decoding on G1") Signed-off-by: Jonas Karlman <jonas@kwiboo.se> --- Changes in v2: - new patch split from "media: hantro: Fix H264 motion vector buffer offset" --- drivers/staging/media/hantro/hantro_g1_h264_dec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)