Messages in this thread | | | Date | Fri, 4 Aug 2023 10:55:23 +0300 | Subject | Re: [v3] media: mediatek: vcodec: fix AV1 decoding on MT8188 | From | Eugen Hristev <> |
| |
Hi Xiaoyong,
On 8/3/23 14:10, Xiaoyong Lu wrote: > Fix AV1 decoding failure when the iova is 36bit. > > Before this fix, the decoder was accessing incorrect addresses with 36bit > iova tile buffer, leading to iommu faults. > > Fixes: 2f5d0aef37c6 ("media: mediatek: vcodec: support stateless AV1 decoder") > Signed-off-by: Xiaoyong Lu<xiaoyong.lu@mediatek.com> > --- > Changes from v2: > > - refine commit subject and message > > Changes from v1: > > - prefer '|' rather than '+' > - prefer '&' rather than shift operation > - add comments for address operations > > v1: > - VDEC HW can access tile buffer and decode normally. > - Test ok by mt8195 32bit and mt8188 36bit iova. > > --- > .../mediatek/vcodec/vdec/vdec_av1_req_lat_if.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_av1_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_av1_req_lat_if.c > index 404a1a23fd402..e9f2393f6a883 100644 > --- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_av1_req_lat_if.c > +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_av1_req_lat_if.c > @@ -1658,9 +1658,9 @@ static void vdec_av1_slice_setup_tile_buffer(struct vdec_av1_slice_instance *ins > u32 allow_update_cdf = 0; > u32 sb_boundary_x_m1 = 0, sb_boundary_y_m1 = 0; > int tile_info_base; > - u32 tile_buf_pa; > + u64 tile_buf_pa; > u32 *tile_info_buf = instance->tile.va; > - u32 pa = (u32)bs->dma_addr; > + u64 pa = (u64)bs->dma_addr;
If it this is a dma address, can't we use dma_addr_t ? isn't it more generic? Or maybe you have a specific reason not to ?
> > if (uh->disable_cdf_update == 0) > allow_update_cdf = 1; > @@ -1673,8 +1673,12 @@ static void vdec_av1_slice_setup_tile_buffer(struct vdec_av1_slice_instance *ins > tile_info_buf[tile_info_base + 0] = (tile_group->tile_size[tile_num] << 3); > tile_buf_pa = pa + tile_group->tile_start_offset[tile_num]; > > - tile_info_buf[tile_info_base + 1] = (tile_buf_pa >> 4) << 4; > - tile_info_buf[tile_info_base + 2] = (tile_buf_pa % 16) << 3; > + /* save av1 tile high 4bits(bit 32-35) address in lower 4 bits position > + * and clear original for hw requirement. > + */ > + tile_info_buf[tile_info_base + 1] = (tile_buf_pa & 0xFFFFFFF0ull) | > + ((tile_buf_pa & 0xF00000000ull) >> 32); > + tile_info_buf[tile_info_base + 2] = (tile_buf_pa & 0xFull) << 3;
Would it be better to use GENMASK if you plan to mask out some of the bits in the tile_buf_pa ?
> > sb_boundary_x_m1 = > (tile->mi_col_starts[tile_col + 1] - tile->mi_col_starts[tile_col] - 1) &
Greetings, Eugen
| |