Hi Paul, > Am 24.01.2021 um 10:43 schrieb Paul Cercueil <p...@crapouillou.net>: > > Hi Nikolaus, > > Le dim. 24 janv. 2021 à 10:30, H. Nikolaus Schaller <h...@goldelico.com> a > écrit : >> Hi Paul, >> we observed the same issue on the jz4730 (which is almost identical >> to the jz4740 wrt. LCDC) and our solution [1] was simpler. >> It leaves the hwdesc f0 and f1 as they are and just takes f1 for really >> programming the first DMA descriptor if there is no OSD. > > Disagreed. With your solution, it ends up using priv->f1 plane with > hwdesc_f0. That's very confusing.
It is a tradeoff between code simplicity and confusion. Indeed difficult to decide. Fortunately I don't have to :) > >> We have tested on jz4730 and jz4780. > > Could I get a tested-by then? :) I'll test with our tree and test both SoC in the next days and give feedback. BR and thanks, Nikolaus > > Cheers, > -Paul > >> Maybe you want to consider that. Then I can officially post it. >> [1] >> https://github.com/goldelico/letux-kernel/commit/3be1de5fdabf2cc1c17f198ded3328cc6e4b9844 >>> Am 24.01.2021 um 09:55 schrieb Paul Cercueil <p...@crapouillou.net>: >>> Even though the JZ4740 did not have the OSD mode, it had (according to >>> the documentation) two DMA channels, but there is absolutely no >>> information about how to select the second DMA channel. >>> Make the ingenic-drm driver work in non-OSD mode by using the >>> foreground0 plane (which is bound to the DMA0 channel) as the primary >>> plane, instead of the foreground1 plane, which is the primary plane >>> when in OSD mode. >>> Fixes: 3c9bea4ef32b ("drm/ingenic: Add support for OSD mode") >>> Cc: <sta...@vger.kernel.org> # v5.8+ >>> Signed-off-by: Paul Cercueil <p...@crapouillou.net> >>> Acked-by: Daniel Vetter <daniel.vet...@ffwll.ch> >>> --- >>> drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 11 +++++++---- >>> 1 file changed, 7 insertions(+), 4 deletions(-) >>> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c >>> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c >>> index b23011c1c5d9..59ce43862e16 100644 >>> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c >>> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c >>> @@ -554,7 +554,7 @@ static void ingenic_drm_plane_atomic_update(struct >>> drm_plane *plane, >>> height = state->src_h >> 16; >>> cpp = state->fb->format->cpp[0]; >>> - if (priv->soc_info->has_osd && plane->type == >>> DRM_PLANE_TYPE_OVERLAY) >>> + if (!priv->soc_info->has_osd || plane->type == >>> DRM_PLANE_TYPE_OVERLAY) >>> hwdesc = &priv->dma_hwdescs->hwdesc_f0; >>> else >>> hwdesc = &priv->dma_hwdescs->hwdesc_f1; >> we just replace this with >> if (priv->soc_info->has_osd && plane->type != >> DRM_PLANE_TYPE_OVERLAY) >> hwdesc = &priv->dma_hwdescs->hwdesc_f1; >> else >> hwdesc = &priv->dma_hwdescs->hwdesc_f0; >> and the remainder can stay as is. >>> @@ -826,6 +826,7 @@ static int ingenic_drm_bind(struct device *dev, bool >>> has_components) >>> const struct jz_soc_info *soc_info; >>> struct ingenic_drm *priv; >>> struct clk *parent_clk; >>> + struct drm_plane *primary; >>> struct drm_bridge *bridge; >>> struct drm_panel *panel; >>> struct drm_encoder *encoder; >>> @@ -940,9 +941,11 @@ static int ingenic_drm_bind(struct device *dev, bool >>> has_components) >>> if (soc_info->has_osd) >>> priv->ipu_plane = drm_plane_from_index(drm, 0); >>> - drm_plane_helper_add(&priv->f1, &ingenic_drm_plane_helper_funcs); >>> + primary = priv->soc_info->has_osd ? &priv->f1 : &priv->f0; >>> - ret = drm_universal_plane_init(drm, &priv->f1, 1, >>> + drm_plane_helper_add(primary, &ingenic_drm_plane_helper_funcs); >>> + >>> + ret = drm_universal_plane_init(drm, primary, 1, >>> &ingenic_drm_primary_plane_funcs, >>> priv->soc_info->formats_f1, >>> priv->soc_info->num_formats_f1, >>> @@ -954,7 +957,7 @@ static int ingenic_drm_bind(struct device *dev, bool >>> has_components) >>> drm_crtc_helper_add(&priv->crtc, &ingenic_drm_crtc_helper_funcs); >>> - ret = drm_crtc_init_with_planes(drm, &priv->crtc, &priv->f1, >>> + ret = drm_crtc_init_with_planes(drm, &priv->crtc, primary, >>> NULL, &ingenic_drm_crtc_funcs, NULL); >>> if (ret) { >>> dev_err(dev, "Failed to init CRTC: %i\n", ret); >>> -- >>> 2.29.2 >> BR and thanks, >> Nikolaus > >
