mbox series

[v2,0/6] media: mediatek: vcodec: Fix 4K decoding support

Message ID 20220706082138.2668163-1-wenst@chromium.org (mailing list archive)
Headers show
Series media: mediatek: vcodec: Fix 4K decoding support | expand

Message

Chen-Yu Tsai July 6, 2022, 8:21 a.m. UTC
This is v2 of the mtk-vcodec 4K decoder fixes.

While testing a backport of recent mtk-vcodec developments on ChromeOS
v5.10 kernel [1], it was found that 4K decoding support had regressed.
The decoder was not correctly reporting 4K frame sizes when queried,
and ChromeOS then determined that the hardware did not support it.

This turned out to be a mix of different bugs:

1. Frame size enumeration on the output side should not depend on the
   currently set format, or any other derived state. This is fixed in
   patch 2.

2. TRY_FMT on the output side was incorrectly clamping the resolution
   based on the current maximum values. It should not. Fixed in patch
   4.

3. The default resolution limit was not set according to the default
   output format determined at runtime, but hard-coded to 1080p. An
   S_FMT call is needed to override this. The instance resolution limit
   is rendered useless after patches 3 and 4, and dropped in patch 5.

Other patches:
- Patch 1 makes stepwise_fhd constant.
- Patch 3 drops redundant aligning of default resolution
- Patch 6 moves framesize inside mtk_video_fmt, making it easier to
  access and removes a list search that was added in patch 4.

Changes since v1:
  - Added patch to const-ify stepwise_fhd, as Nicolas requested.
  - Dropped old patch 2 (media: mediatek: vcodec: dec: Set default
    max resolution based on format)
  - Dropped old patch 4 (media: mediatek: vcodec: dec: Set maximum
    resolution when S_FMT on output only)
  - Made max resolution lookup for TRY_FMT return stepwise structure in
    patch 4, which helps with the last patch that moves framesize
    stepwise into mtk_video_fmt.
  - Did some style cleanups in patch 4

This series is based on next-20220705.

This was only tested on the backport kernel [1] on MT8195, which is the
only currently supported SoC that does 4K decoding. Hopefully the folks
at Collabora can give this a test on their mainline MT8195 integration
branch.


Regards
ChenYu

[1] https://crrev.com/c/3713491

Chen-Yu Tsai (6):
  media: mediatek: vcodec: decoder: Const-ify stepwise_fhd
  media: mediatek: vcodec: decoder: Fix 4K frame size enumeration
  media: mediatek: vcodec: decoder: Skip alignment for default
    resolution
  media: mediatek: vcodec: decoder: Fix resolution clamping in TRY_FMT
  media: mediatek: vcodec: decoder: Drop max_{width,height} from
    mtk_vcodec_ctx
  media: mediatek: vcodec: decoder: Embed framesize inside mtk_video_fmt

 .../platform/mediatek/vcodec/mtk_vcodec_dec.c | 54 ++++++++-----------
 .../mediatek/vcodec/mtk_vcodec_dec_stateful.c | 29 +++-------
 .../vcodec/mtk_vcodec_dec_stateless.c         | 30 +++++------
 .../platform/mediatek/vcodec/mtk_vcodec_drv.h | 20 +------
 4 files changed, 41 insertions(+), 92 deletions(-)

Comments

AngeloGioacchino Del Regno July 6, 2022, 9:53 a.m. UTC | #1
Il 06/07/22 10:21, Chen-Yu Tsai ha scritto:
> This is v2 of the mtk-vcodec 4K decoder fixes.
> 
> While testing a backport of recent mtk-vcodec developments on ChromeOS
> v5.10 kernel [1], it was found that 4K decoding support had regressed.
> The decoder was not correctly reporting 4K frame sizes when queried,
> and ChromeOS then determined that the hardware did not support it.
> 
> This turned out to be a mix of different bugs:
> 
> 1. Frame size enumeration on the output side should not depend on the
>     currently set format, or any other derived state. This is fixed in
>     patch 2.
> 
> 2. TRY_FMT on the output side was incorrectly clamping the resolution
>     based on the current maximum values. It should not. Fixed in patch
>     4.
> 
> 3. The default resolution limit was not set according to the default
>     output format determined at runtime, but hard-coded to 1080p. An
>     S_FMT call is needed to override this. The instance resolution limit
>     is rendered useless after patches 3 and 4, and dropped in patch 5.
> 
> Other patches:
> - Patch 1 makes stepwise_fhd constant.
> - Patch 3 drops redundant aligning of default resolution
> - Patch 6 moves framesize inside mtk_video_fmt, making it easier to
>    access and removes a list search that was added in patch 4.
> 
> Changes since v1:
>    - Added patch to const-ify stepwise_fhd, as Nicolas requested.
>    - Dropped old patch 2 (media: mediatek: vcodec: dec: Set default
>      max resolution based on format)
>    - Dropped old patch 4 (media: mediatek: vcodec: dec: Set maximum
>      resolution when S_FMT on output only)
>    - Made max resolution lookup for TRY_FMT return stepwise structure in
>      patch 4, which helps with the last patch that moves framesize
>      stepwise into mtk_video_fmt.
>    - Did some style cleanups in patch 4
> 
> This series is based on next-20220705.
> 
> This was only tested on the backport kernel [1] on MT8195, which is the
> only currently supported SoC that does 4K decoding. Hopefully the folks
> at Collabora can give this a test on their mainline MT8195 integration
> branch.

Cannot spot any regression after fluster tests.

For the whole series:

Tested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> 
> 
> Regards
> ChenYu
> 
> [1] https://crrev.com/c/3713491
> 
> Chen-Yu Tsai (6):
>    media: mediatek: vcodec: decoder: Const-ify stepwise_fhd
>    media: mediatek: vcodec: decoder: Fix 4K frame size enumeration
>    media: mediatek: vcodec: decoder: Skip alignment for default
>      resolution
>    media: mediatek: vcodec: decoder: Fix resolution clamping in TRY_FMT
>    media: mediatek: vcodec: decoder: Drop max_{width,height} from
>      mtk_vcodec_ctx
>    media: mediatek: vcodec: decoder: Embed framesize inside mtk_video_fmt
> 
>   .../platform/mediatek/vcodec/mtk_vcodec_dec.c | 54 ++++++++-----------
>   .../mediatek/vcodec/mtk_vcodec_dec_stateful.c | 29 +++-------
>   .../vcodec/mtk_vcodec_dec_stateless.c         | 30 +++++------
>   .../platform/mediatek/vcodec/mtk_vcodec_drv.h | 20 +------
>   4 files changed, 41 insertions(+), 92 deletions(-)
>