| Message ID | 20191105211034.123937-8-sean@poorly.run (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | df444457711858d398896d678bd3cd7f43f1c1f5 |
| Headers | show |
| Series | drm/mediatek: Refactor plane init/check and support rotation | expand |
Hi, Sean: On Tue, 2019-11-05 at 16:10 -0500, Sean Paul wrote: > From: Sean Paul <seanpaul@chromium.org> > > Now that we support both reflections, we can expose 180 degree rotation > and rely on the simplify routine to convert that into REFLECT_X | > REFLECT_Y > Patch 1 ~ 6 of this series looks good to me. For this one, I think the rotation check in mtk_ovl_layer_check() should be modified. Regards, CK > Signed-off-by: Sean Paul <seanpaul@chromium.org> > --- > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > index f4c4d3fedc5f..4a55bb6e2213 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > @@ -143,8 +143,8 @@ static unsigned int mtk_ovl_layer_nr(struct mtk_ddp_comp *comp) > > static unsigned int mtk_ovl_supported_rotations(struct mtk_ddp_comp *comp) > { > - return DRM_MODE_ROTATE_0 | DRM_MODE_REFLECT_Y | > - DRM_MODE_REFLECT_X; > + return DRM_MODE_ROTATE_0 | DRM_MODE_ROTATE_180 | > + DRM_MODE_REFLECT_X | DRM_MODE_REFLECT_Y; > } > > static int mtk_ovl_layer_check(struct mtk_ddp_comp *comp, unsigned int idx,
On Wed, Nov 6, 2019 at 4:07 AM CK Hu <ck.hu@mediatek.com> wrote: > > Hi, Sean: > > On Tue, 2019-11-05 at 16:10 -0500, Sean Paul wrote: > > From: Sean Paul <seanpaul@chromium.org> > > > > Now that we support both reflections, we can expose 180 degree rotation > > and rely on the simplify routine to convert that into REFLECT_X | > > REFLECT_Y > > > > Patch 1 ~ 6 of this series looks good to me. > For this one, I think the rotation check in mtk_ovl_layer_check() should > be modified. > Thanks for the quick reviews, CK! In what way would you like the check to be modified? I'm guessing you're asking that I add it to the bitmask passed to the simplify_rotation function? If that's the case, we don't actually want to modify the simplify_rotation bitmask since that bitmask is supposed to be only the rotations that the hardware can achieve. So if you pass ROTATE_180 into simplify_rotation, it will return REFLECT_X | REFLECT_Y, which is what we want. Does that make sense, or am I missing something? Thanks, Sean > Regards, > CK > > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > > --- > > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > index f4c4d3fedc5f..4a55bb6e2213 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > @@ -143,8 +143,8 @@ static unsigned int mtk_ovl_layer_nr(struct mtk_ddp_comp *comp) > > > > static unsigned int mtk_ovl_supported_rotations(struct mtk_ddp_comp *comp) > > { > > - return DRM_MODE_ROTATE_0 | DRM_MODE_REFLECT_Y | > > - DRM_MODE_REFLECT_X; > > + return DRM_MODE_ROTATE_0 | DRM_MODE_ROTATE_180 | > > + DRM_MODE_REFLECT_X | DRM_MODE_REFLECT_Y; > > } > > > > static int mtk_ovl_layer_check(struct mtk_ddp_comp *comp, unsigned int idx, > >
Hi, Sean: On Wed, 2019-11-06 at 09:42 -0500, Sean Paul wrote: > On Wed, Nov 6, 2019 at 4:07 AM CK Hu <ck.hu@mediatek.com> wrote: > > > > Hi, Sean: > > > > On Tue, 2019-11-05 at 16:10 -0500, Sean Paul wrote: > > > From: Sean Paul <seanpaul@chromium.org> > > > > > > Now that we support both reflections, we can expose 180 degree rotation > > > and rely on the simplify routine to convert that into REFLECT_X | > > > REFLECT_Y > > > > > > > Patch 1 ~ 6 of this series looks good to me. > > For this one, I think the rotation check in mtk_ovl_layer_check() should > > be modified. > > > > Thanks for the quick reviews, CK! > > In what way would you like the check to be modified? I'm guessing > you're asking that I add it to the bitmask passed to the > simplify_rotation function? > > If that's the case, we don't actually want to modify the > simplify_rotation bitmask since that bitmask is supposed to be only > the rotations that the hardware can achieve. So if you pass ROTATE_180 > into simplify_rotation, it will return REFLECT_X | REFLECT_Y, which is > what we want. Does that make sense, or am I missing something? drm_rotation_simplify() is really hard to understand. You're right, so apply this series to mediatek-drm-next-5.5-2 [1], thanks. [1] https://github.com/ckhu-mediatek/linux.git-tags/commits/mediatek-drm-next-5.5-2 Regards, CK > > Thanks, > > Sean > > > Regards, > > CK > > > > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > > > --- > > > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > > index f4c4d3fedc5f..4a55bb6e2213 100644 > > > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > > @@ -143,8 +143,8 @@ static unsigned int mtk_ovl_layer_nr(struct mtk_ddp_comp *comp) > > > > > > static unsigned int mtk_ovl_supported_rotations(struct mtk_ddp_comp *comp) > > > { > > > - return DRM_MODE_ROTATE_0 | DRM_MODE_REFLECT_Y | > > > - DRM_MODE_REFLECT_X; > > > + return DRM_MODE_ROTATE_0 | DRM_MODE_ROTATE_180 | > > > + DRM_MODE_REFLECT_X | DRM_MODE_REFLECT_Y; > > > } > > > > > > static int mtk_ovl_layer_check(struct mtk_ddp_comp *comp, unsigned int idx, > > > >
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index f4c4d3fedc5f..4a55bb6e2213 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -143,8 +143,8 @@ static unsigned int mtk_ovl_layer_nr(struct mtk_ddp_comp *comp) static unsigned int mtk_ovl_supported_rotations(struct mtk_ddp_comp *comp) { - return DRM_MODE_ROTATE_0 | DRM_MODE_REFLECT_Y | - DRM_MODE_REFLECT_X; + return DRM_MODE_ROTATE_0 | DRM_MODE_ROTATE_180 | + DRM_MODE_REFLECT_X | DRM_MODE_REFLECT_Y; } static int mtk_ovl_layer_check(struct mtk_ddp_comp *comp, unsigned int idx,