Messages in this thread Patch in this message | | | From | Shawn Sung (宋孝謙) <> | Subject | Re: [PATCH v4 1/1] drm/mediatek: Fix errors when reporting rotation capability | Date | Tue, 19 Dec 2023 03:45:43 +0000 |
| |
Hi CK,
On Tue, 2023-12-19 at 02:35 +0000, CK Hu (胡俊光) wrote: > Hi, Hsiao-chien: > > On Fri, 2023-11-24 at 18:00 +0800, Hsiao Chien Sung wrote: > > Create rotation property according to the hardware capability. > > Since currently OVL of all chips support same rotation, > > no need to define it in the driver data. > > > > Fixes: 84d805753983 ("drm/mediatek: Support reflect-y plane > > rotation") > > > > Reviewed-by: AngeloGioacchino Del Regno < > > angelogioacchino.delregno@collabora.com> > > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com> > > --- > > drivers/gpu/drm/mediatek/mtk_disp_drv.h | 1 + > > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 18 ++++++-------- > > ---- > > .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 9 +++++++++ > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 1 + > > drivers/gpu/drm/mediatek/mtk_drm_plane.c | 2 +- > > 5 files changed, 18 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > > index 4d6e8b667bc3..c5afeb7c5527 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > > @@ -127,6 +127,7 @@ void mtk_ovl_adaptor_register_vblank_cb(struct > > device *dev, void (*vblank_cb)(vo > > void mtk_ovl_adaptor_unregister_vblank_cb(struct device *dev); > > void mtk_ovl_adaptor_enable_vblank(struct device *dev); > > void mtk_ovl_adaptor_disable_vblank(struct device *dev); > > +unsigned int mtk_ovl_adaptor_supported_rotations(struct device > > *dev); > > void mtk_ovl_adaptor_start(struct device *dev); > > void mtk_ovl_adaptor_stop(struct device *dev); > > unsigned int mtk_ovl_adaptor_layer_nr(struct device *dev); > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > index ecc38932fd44..319bbfde5cf9 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > @@ -415,6 +415,10 @@ unsigned int mtk_ovl_layer_nr(struct device > > *dev) > > > > unsigned int mtk_ovl_supported_rotations(struct device *dev) > > { > > + /* > > + * although currently OVL can only do reflection, > > + * reflect x + reflect y = rotate 180 > > + */ > > return DRM_MODE_ROTATE_0 | DRM_MODE_ROTATE_180 | > > DRM_MODE_REFLECT_X | DRM_MODE_REFLECT_Y; > > } > > @@ -423,27 +427,17 @@ int mtk_ovl_layer_check(struct device *dev, > > unsigned int idx, > > struct mtk_plane_state *mtk_state) > > { > > struct drm_plane_state *state = &mtk_state->base; > > - unsigned int rotation = 0; > > > > - rotation = drm_rotation_simplify(state->rotation, > > - DRM_MODE_ROTATE_0 | > > - DRM_MODE_REFLECT_X | > > - DRM_MODE_REFLECT_Y); > > - rotation &= ~DRM_MODE_ROTATE_0; > > - > > - /* We can only do reflection, not rotation */ > > - if ((rotation & DRM_MODE_ROTATE_MASK) != 0) > > + if (state->rotation & ~mtk_ovl_supported_rotations(dev)) > > return -EINVAL; > > The commit message of this patch is "Create rotation property > according > to the hardware capability". I think this modification is not related > to create property, so separate this to another patch.
Since these modifications are all related to rotation property, or should I change the title and commit message to "Modify roation property for passing IGT"?
> > > > > /* > > * TODO: Rotating/reflecting YUV buffers is not supported at > > this time. > > * Only RGB[AX] variants are supported. > > */ > > - if (state->fb->format->is_yuv && rotation != 0) > > + if (state->fb->format->is_yuv && (state->rotation & > > ~DRM_MODE_ROTATE_0)) > > return -EINVAL; > > > > - state->rotation = rotation; > > - > > return 0; > > } > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > > b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > > index 4398db9a6276..273c79d37bef 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > > @@ -383,6 +383,15 @@ void mtk_ovl_adaptor_register_vblank_cb(struct > > device *dev, void (*vblank_cb)(vo > > vblank_cb, vblank_cb_data); > > } > > > > +unsigned int mtk_ovl_adaptor_supported_rotations(struct device > > *dev) > > +{ > > + /* > > + * should still return DRM_MODE_ROTATE_0 if rotation is not > > supported, > > + * or IGT will fail. > > + */ > > + return DRM_MODE_ROTATE_0; > > +} > > + > > void mtk_ovl_adaptor_unregister_vblank_cb(struct device *dev) > > { > > struct mtk_disp_ovl_adaptor *ovl_adaptor = > > dev_get_drvdata(dev); > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > index ffa4868b1222..206dd6f6f99e 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > @@ -422,6 +422,7 @@ static const struct mtk_ddp_comp_funcs > > ddp_ovl_adaptor = { > > .remove = mtk_ovl_adaptor_remove_comp, > > .get_formats = mtk_ovl_adaptor_get_formats, > > .get_num_formats = mtk_ovl_adaptor_get_num_formats, > > + .supported_rotations = mtk_ovl_adaptor_supported_rotations, > > }; > > > > static const char * const mtk_ddp_comp_stem[MTK_DDP_COMP_TYPE_MAX] > > = > > { > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > > b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > > index e2ec61b69618..894c39a38a58 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > > @@ -344,7 +344,7 @@ int mtk_plane_init(struct drm_device *dev, > > struct > > drm_plane *plane, > > return err; > > } > > > > - if (supported_rotations & ~DRM_MODE_ROTATE_0) { > > + if (supported_rotations) { > > Why need this modification? > Before Sean's patch [1], only support rotate 0 and does not create > property and it works. Why does it must create property for only > support rotate 0?
Yes, as long as the user doesn't check rotation properties before commiting the pitures, there will be no problem. But IGT somehow checked this DRM_MODE_ROTATE_0 flag before executing the tests (not all of them), and leads to failures in these test itmes.
> > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek/mtk_drm_plane.c?h=v6.7-rc6&id=ef87d3e2dd251374c5c9fa3b6502aeff8fe29da9 > > Regards, > CK > > > err = drm_plane_create_rotation_property(plane, > > DRM_MODE_ROTAT > > E_0, > > supported_rota > > tions); > > -- > > 2.39.2 > >
Thanks, Shawn
| |