Am Dienstag, dem 08.11.2022 um 15:14 +0100 schrieb Philipp Zabel:
> ipu_src_rect_width() was introduced to support odd screen resolutions
> such as 1366x768 by internally rounding up primary plane width to a
> multiple of 8 and compensating with reduced horizontal blanking.
> This also caused overlay plane width to be rounded up, which was not
> intended. Fix overlay plane width by limiting the rounding up to the
> primary plane.
>
> drm_rect_width(&new_state->src) >> 16 is the same value as
> drm_rect_width(dst) because there is no plane scaling support.
Heh, I stumbled at exactly this point while reading the code changes
and was about to suggest it be added to the changelog until I realized
that it's already here. :)
>
> Fixes: 94dfec48fca7 ("drm/imx: Add 8 pixel alignment fix")
> Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
Reviewed-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
One note below.
> ---
> drivers/gpu/drm/imx/ipuv3-plane.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
> index dba4f7d81d69..80142d9a4a55 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -614,6 +614,11 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
> break;
> }
>
> + if (ipu_plane->dp_flow == IPU_DP_FLOW_SYNC_BG)
> + width = ipu_src_rect_width(new_state);
> + else
> + width = drm_rect_width(&new_state->src) >> 16;
> +
> eba = drm_plane_state_to_eba(new_state, 0);
>
> /*
> @@ -622,8 +627,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
> */
> if (ipu_state->use_pre) {
> axi_id = ipu_chan_assign_axi_id(ipu_plane->dma);
> - ipu_prg_channel_configure(ipu_plane->ipu_ch, axi_id,
> - ipu_src_rect_width(new_state),
> + ipu_prg_channel_configure(ipu_plane->ipu_ch, axi_id, width,
> drm_rect_height(&new_state->src) >> 16,
> fb->pitches[0], fb->format->format,
> fb->modifier, &eba);
> @@ -678,9 +682,8 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
> break;
> }
>
> - ipu_dmfc_config_wait4eot(ipu_plane->dmfc, ALIGN(drm_rect_width(dst), 8));
> + ipu_dmfc_config_wait4eot(ipu_plane->dmfc, width);
>
> - width = ipu_src_rect_width(new_state);
> height = drm_rect_height(&new_state->src) >> 16;
There's a opportunity for a follow-up cleanup here:
"drm_rect_height(&new_state->src) >> 16" is used multiple times in this
function, which could be replaced by using this local height variable
instead.
> info = drm_format_info(fb->format->format);
> ipu_calculate_bursts(width, info->cpp[0], fb->pitches[0],
> @@ -744,8 +747,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
> ipu_cpmem_set_burstsize(ipu_plane->ipu_ch, 16);
>
> ipu_cpmem_zero(ipu_plane->alpha_ch);
> - ipu_cpmem_set_resolution(ipu_plane->alpha_ch,
> - ipu_src_rect_width(new_state),
> + ipu_cpmem_set_resolution(ipu_plane->alpha_ch, width,
> drm_rect_height(&new_state->src) >> 16);
> ipu_cpmem_set_format_passthrough(ipu_plane->alpha_ch, 8);
> ipu_cpmem_set_high_priority(ipu_plane->alpha_ch);
>
> base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780
