Hi,
Here is a set of 11 patches for the ingenic-drm driver.
Patches 1-7 are mostly generic cleanups, which will grease up the way for bigger changes to be introduced.
Patch 3 adds support for a private state structure, which is then used to store state-specific information, which was previously stored in the driver's private structure directly.
Patch 10 is the big one; it adds a double-scan feature emulated with DMA descriptors. This trick makes it possible to support a handful of boards which have strange panels with non-square pixels (320x480 4:3).
Patch 11 updates the driver to support one top-level bridge per encoder, as it seems to be the norm now.
Cheers, -Paul
Paul Cercueil (11): drm/ingenic: Remove dead code drm/ingenic: Simplify code by using hwdescs array drm/ingenic: Add support for private objects drm/ingenic: Move no_vblank to private state drm/ingenic: Move IPU scale settings to private state drm/ingenic: Set DMA descriptor chain register when starting CRTC drm/ingenic: Upload palette before frame drm/ingenic: Support custom GEM object drm/ingenic: Add ingenic_drm_gem_fb_destroy() function drm/ingenic: Add doublescan feature drm/ingenic: Attach bridge chain to encoders
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 414 ++++++++++++++++++---- drivers/gpu/drm/ingenic/ingenic-ipu.c | 127 ++++++- 2 files changed, 458 insertions(+), 83 deletions(-)
The priv->ipu_plane would get a different value further down the code, without the first assigned value being read first; so the first assignation can be dropped.
Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index 5244f4763477..93c099e7464d 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -988,9 +988,6 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) priv->dma_hwdescs->hwdesc_pal.cmd = JZ_LCD_CMD_ENABLE_PAL | (sizeof(priv->dma_hwdescs->palette) / 4);
- if (soc_info->has_osd) - priv->ipu_plane = drm_plane_from_index(drm, 0); - primary = priv->soc_info->has_osd ? &priv->f1 : &priv->f0;
drm_plane_helper_add(primary, &ingenic_drm_plane_helper_funcs);
Instead of having one 'hwdesc' variable for the plane #0 and one for the plane #1, use a 'hwdesc[2]' array, where the DMA hardware descriptors are indexed by the plane's number.
Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 38 ++++++++++++----------- 1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index 93c099e7464d..4e41bdf2f3fd 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -50,8 +50,7 @@ struct ingenic_dma_hwdesc { } __aligned(16);
struct ingenic_dma_hwdescs { - struct ingenic_dma_hwdesc hwdesc_f0; - struct ingenic_dma_hwdesc hwdesc_f1; + struct ingenic_dma_hwdesc hwdesc[2]; struct ingenic_dma_hwdesc hwdesc_pal; u16 palette[256] __aligned(16); }; @@ -142,6 +141,13 @@ static inline struct ingenic_drm *drm_nb_get_priv(struct notifier_block *nb) return container_of(nb, struct ingenic_drm, clock_nb); }
+static inline dma_addr_t dma_hwdesc_addr(const struct ingenic_drm *priv, bool use_f1) +{ + u32 offset = offsetof(struct ingenic_dma_hwdescs, hwdesc[use_f1]); + + return priv->dma_hwdescs_phys + offset; +} + static int ingenic_drm_update_pixclk(struct notifier_block *nb, unsigned long action, void *data) @@ -563,6 +569,7 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, struct ingenic_dma_hwdesc *hwdesc; unsigned int width, height, cpp, offset; dma_addr_t addr; + bool use_f1; u32 fourcc;
if (newstate && newstate->fb) { @@ -570,16 +577,14 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, drm_fb_cma_sync_non_coherent(&priv->drm, oldstate, newstate);
crtc_state = newstate->crtc->state; + use_f1 = priv->soc_info->has_osd && plane != &priv->f0;
addr = drm_fb_cma_get_gem_addr(newstate->fb, newstate, 0); width = newstate->src_w >> 16; height = newstate->src_h >> 16; cpp = newstate->fb->format->cpp[0];
- if (!priv->soc_info->has_osd || plane == &priv->f0) - hwdesc = &priv->dma_hwdescs->hwdesc_f0; - else - hwdesc = &priv->dma_hwdescs->hwdesc_f1; + hwdesc = &priv->dma_hwdescs->hwdesc[use_f1];
hwdesc->addr = addr; hwdesc->cmd = JZ_LCD_CMD_EOF_IRQ | (width * height * cpp / 4); @@ -592,9 +597,9 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, if (fourcc == DRM_FORMAT_C8) offset = offsetof(struct ingenic_dma_hwdescs, hwdesc_pal); else - offset = offsetof(struct ingenic_dma_hwdescs, hwdesc_f0); + offset = offsetof(struct ingenic_dma_hwdescs, hwdesc[0]);
- priv->dma_hwdescs->hwdesc_f0.next = priv->dma_hwdescs_phys + offset; + priv->dma_hwdescs->hwdesc[0].next = priv->dma_hwdescs_phys + offset;
crtc_state->color_mgmt_changed = fourcc == DRM_FORMAT_C8; } @@ -968,20 +973,17 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
/* Configure DMA hwdesc for foreground0 plane */ - dma_hwdesc_phys_f0 = priv->dma_hwdescs_phys - + offsetof(struct ingenic_dma_hwdescs, hwdesc_f0); - priv->dma_hwdescs->hwdesc_f0.next = dma_hwdesc_phys_f0; - priv->dma_hwdescs->hwdesc_f0.id = 0xf0; + dma_hwdesc_phys_f0 = dma_hwdesc_addr(priv, 0); + priv->dma_hwdescs->hwdesc[0].next = dma_hwdesc_phys_f0; + priv->dma_hwdescs->hwdesc[0].id = 0xf0;
/* Configure DMA hwdesc for foreground1 plane */ - dma_hwdesc_phys_f1 = priv->dma_hwdescs_phys - + offsetof(struct ingenic_dma_hwdescs, hwdesc_f1); - priv->dma_hwdescs->hwdesc_f1.next = dma_hwdesc_phys_f1; - priv->dma_hwdescs->hwdesc_f1.id = 0xf1; + dma_hwdesc_phys_f1 = dma_hwdesc_addr(priv, 1); + priv->dma_hwdescs->hwdesc[1].next = dma_hwdesc_phys_f1; + priv->dma_hwdescs->hwdesc[1].id = 0xf1;
/* Configure DMA hwdesc for palette */ - priv->dma_hwdescs->hwdesc_pal.next = priv->dma_hwdescs_phys - + offsetof(struct ingenic_dma_hwdescs, hwdesc_f0); + priv->dma_hwdescs->hwdesc_pal.next = dma_hwdesc_phys_f0; priv->dma_hwdescs->hwdesc_pal.id = 0xc0; priv->dma_hwdescs->hwdesc_pal.addr = priv->dma_hwdescs_phys + offsetof(struct ingenic_dma_hwdescs, palette);
Until now, the ingenic-drm as well as the ingenic-ipu drivers used to put state-specific information in their respective private structure.
Add boilerplate code to support private objects in the two drivers, so that state-specific information can be put in the state-specific private structure.
Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 61 +++++++++++++++++++++++ drivers/gpu/drm/ingenic/ingenic-ipu.c | 54 ++++++++++++++++++++ 2 files changed, 115 insertions(+)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index 4e41bdf2f3fd..e81084eb3b0e 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -64,6 +64,10 @@ struct jz_soc_info { unsigned int num_formats_f0, num_formats_f1; };
+struct ingenic_drm_private_state { + struct drm_private_state base; +}; + struct ingenic_drm { struct drm_device drm; /* @@ -99,8 +103,16 @@ struct ingenic_drm { struct mutex clk_mutex; bool update_clk_rate; struct notifier_block clock_nb; + + struct drm_private_obj private_obj; };
+static inline struct ingenic_drm_private_state * +to_ingenic_drm_priv_state(struct drm_private_state *state) +{ + return container_of(state, struct ingenic_drm_private_state, base); +} + static bool ingenic_drm_writeable_reg(struct device *dev, unsigned int reg) { switch (reg) { @@ -790,6 +802,28 @@ ingenic_drm_gem_create_object(struct drm_device *drm, size_t size) return &obj->base; }
+static struct drm_private_state * +ingenic_drm_duplicate_state(struct drm_private_obj *obj) +{ + struct ingenic_drm_private_state *state = to_ingenic_drm_priv_state(obj->state); + + state = kmemdup(state, sizeof(*state), GFP_KERNEL); + if (!state) + return NULL; + + __drm_atomic_helper_private_obj_duplicate_state(obj, &state->base); + + return &state->base; +} + +static void ingenic_drm_destroy_state(struct drm_private_obj *obj, + struct drm_private_state *state) +{ + struct ingenic_drm_private_state *priv_state = to_ingenic_drm_priv_state(state); + + kfree(priv_state); +} + DEFINE_DRM_GEM_CMA_FOPS(ingenic_drm_fops);
static const struct drm_driver ingenic_drm_driver_data = { @@ -863,6 +897,11 @@ static struct drm_mode_config_helper_funcs ingenic_drm_mode_config_helpers = { .atomic_commit_tail = ingenic_drm_atomic_helper_commit_tail, };
+static const struct drm_private_state_funcs ingenic_drm_private_state_funcs = { + .atomic_duplicate_state = ingenic_drm_duplicate_state, + .atomic_destroy_state = ingenic_drm_destroy_state, +}; + static void ingenic_drm_unbind_all(void *d) { struct ingenic_drm *priv = d; @@ -875,9 +914,15 @@ static void __maybe_unused ingenic_drm_release_rmem(void *d) of_reserved_mem_device_release(d); }
+static void ingenic_drm_atomic_private_obj_fini(struct drm_device *drm, void *private_obj) +{ + drm_atomic_private_obj_fini(private_obj); +} + static int ingenic_drm_bind(struct device *dev, bool has_components) { struct platform_device *pdev = to_platform_device(dev); + struct ingenic_drm_private_state *private_state; const struct jz_soc_info *soc_info; struct ingenic_drm *priv; struct clk *parent_clk; @@ -1158,6 +1203,20 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) goto err_devclk_disable; }
+ private_state = kzalloc(sizeof(*private_state), GFP_KERNEL); + if (!private_state) { + ret = -ENOMEM; + goto err_clk_notifier_unregister; + } + + drm_atomic_private_obj_init(drm, &priv->private_obj, &private_state->base, + &ingenic_drm_private_state_funcs); + + ret = drmm_add_action_or_reset(drm, ingenic_drm_atomic_private_obj_fini, + &priv->private_obj); + if (ret) + goto err_private_state_free; + ret = drm_dev_register(drm, 0); if (ret) { dev_err(dev, "Failed to register DRM driver\n"); @@ -1168,6 +1227,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
return 0;
+err_private_state_free: + kfree(private_state); err_clk_notifier_unregister: clk_notifier_unregister(parent_clk, &priv->clock_nb); err_devclk_disable: diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c index 61b6d9fdbba1..007cd547b285 100644 --- a/drivers/gpu/drm/ingenic/ingenic-ipu.c +++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c @@ -45,6 +45,10 @@ struct soc_info { unsigned int weight, unsigned int offset); };
+struct ingenic_ipu_private_state { + struct drm_private_state base; +}; + struct ingenic_ipu { struct drm_plane plane; struct drm_device *drm; @@ -60,6 +64,8 @@ struct ingenic_ipu {
struct drm_property *sharpness_prop; unsigned int sharpness; + + struct drm_private_obj private_obj; };
/* Signed 15.16 fixed-point math (for bicubic scaling coefficients) */ @@ -73,6 +79,12 @@ static inline struct ingenic_ipu *plane_to_ingenic_ipu(struct drm_plane *plane) return container_of(plane, struct ingenic_ipu, plane); }
+static inline struct ingenic_ipu_private_state * +to_ingenic_ipu_priv_state(struct drm_private_state *state) +{ + return container_of(state, struct ingenic_ipu_private_state, base); +} + /* * Apply conventional cubic convolution kernel. Both parameters * and return value are 15.16 signed fixed-point. @@ -680,6 +692,33 @@ static const struct drm_plane_funcs ingenic_ipu_plane_funcs = { .atomic_set_property = ingenic_ipu_plane_atomic_set_property, };
+static struct drm_private_state * +ingenic_ipu_duplicate_state(struct drm_private_obj *obj) +{ + struct ingenic_ipu_private_state *state = to_ingenic_ipu_priv_state(obj->state); + + state = kmemdup(state, sizeof(*state), GFP_KERNEL); + if (!state) + return NULL; + + __drm_atomic_helper_private_obj_duplicate_state(obj, &state->base); + + return &state->base; +} + +static void ingenic_ipu_destroy_state(struct drm_private_obj *obj, + struct drm_private_state *state) +{ + struct ingenic_ipu_private_state *priv_state = to_ingenic_ipu_priv_state(state); + + kfree(priv_state); +} + +static const struct drm_private_state_funcs ingenic_ipu_private_state_funcs = { + .atomic_duplicate_state = ingenic_ipu_duplicate_state, + .atomic_destroy_state = ingenic_ipu_destroy_state, +}; + static irqreturn_t ingenic_ipu_irq_handler(int irq, void *arg) { struct ingenic_ipu *ipu = arg; @@ -718,6 +757,7 @@ static const struct regmap_config ingenic_ipu_regmap_config = { static int ingenic_ipu_bind(struct device *dev, struct device *master, void *d) { struct platform_device *pdev = to_platform_device(dev); + struct ingenic_ipu_private_state *private_state; const struct soc_info *soc_info; struct drm_device *drm = d; struct drm_plane *plane; @@ -811,7 +851,20 @@ static int ingenic_ipu_bind(struct device *dev, struct device *master, void *d) return err; }
+ private_state = kzalloc(sizeof(*private_state), GFP_KERNEL); + if (!private_state) { + err = -ENOMEM; + goto err_clk_unprepare; + } + + drm_atomic_private_obj_init(drm, &ipu->private_obj, &private_state->base, + &ingenic_ipu_private_state_funcs); + return 0; + +err_clk_unprepare: + clk_unprepare(ipu->clk); + return err; }
static void ingenic_ipu_unbind(struct device *dev, @@ -819,6 +872,7 @@ static void ingenic_ipu_unbind(struct device *dev, { struct ingenic_ipu *ipu = dev_get_drvdata(dev);
+ drm_atomic_private_obj_fini(&ipu->private_obj); clk_unprepare(ipu->clk); }
This information is carried from the ".atomic_check" to the ".atomic_commit_tail"; as such it is state-specific, and should be moved to the private state structure.
Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 41 ++++++++++++++++++++--- 1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index e81084eb3b0e..639994329c60 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -66,6 +66,8 @@ struct jz_soc_info {
struct ingenic_drm_private_state { struct drm_private_state base; + + bool no_vblank; };
struct ingenic_drm { @@ -87,7 +89,6 @@ struct ingenic_drm { dma_addr_t dma_hwdescs_phys;
bool panel_is_sharp; - bool no_vblank;
/* * clk_mutex is used to synchronize the pixel clock rate update with @@ -113,6 +114,30 @@ to_ingenic_drm_priv_state(struct drm_private_state *state) return container_of(state, struct ingenic_drm_private_state, base); }
+static struct ingenic_drm_private_state * +ingenic_drm_get_priv_state(struct ingenic_drm *priv, struct drm_atomic_state *state) +{ + struct drm_private_state *priv_state; + + priv_state = drm_atomic_get_private_obj_state(state, &priv->private_obj); + if (IS_ERR(priv_state)) + return ERR_CAST(priv_state); + + return to_ingenic_drm_priv_state(priv_state); +} + +static struct ingenic_drm_private_state * +ingenic_drm_get_new_priv_state(struct ingenic_drm *priv, struct drm_atomic_state *state) +{ + struct drm_private_state *priv_state; + + priv_state = drm_atomic_get_new_private_obj_state(state, &priv->private_obj); + if (!priv_state) + return NULL; + + return to_ingenic_drm_priv_state(priv_state); +} + static bool ingenic_drm_writeable_reg(struct device *dev, unsigned int reg) { switch (reg) { @@ -268,6 +293,7 @@ static int ingenic_drm_crtc_atomic_check(struct drm_crtc *crtc, crtc); struct ingenic_drm *priv = drm_crtc_get_priv(crtc); struct drm_plane_state *f1_state, *f0_state, *ipu_state = NULL; + struct ingenic_drm_private_state *priv_state;
if (crtc_state->gamma_lut && drm_color_lut_size(crtc_state->gamma_lut) != ARRAY_SIZE(priv->dma_hwdescs->palette)) { @@ -299,9 +325,13 @@ static int ingenic_drm_crtc_atomic_check(struct drm_crtc *crtc, } }
+ priv_state = ingenic_drm_get_priv_state(priv, state); + if (IS_ERR(priv_state)) + return PTR_ERR(priv_state); + /* If all the planes are disabled, we won't get a VBLANK IRQ */ - priv->no_vblank = !f1_state->fb && !f0_state->fb && - !(ipu_state && ipu_state->fb); + priv_state->no_vblank = !f1_state->fb && !f0_state->fb && + !(ipu_state && ipu_state->fb); }
return 0; @@ -727,6 +757,7 @@ static void ingenic_drm_atomic_helper_commit_tail(struct drm_atomic_state *old_s */ struct drm_device *dev = old_state->dev; struct ingenic_drm *priv = drm_device_get_priv(dev); + struct ingenic_drm_private_state *priv_state;
drm_atomic_helper_commit_modeset_disables(dev, old_state);
@@ -736,7 +767,9 @@ static void ingenic_drm_atomic_helper_commit_tail(struct drm_atomic_state *old_s
drm_atomic_helper_commit_hw_done(old_state);
- if (!priv->no_vblank) + priv_state = ingenic_drm_get_new_priv_state(priv, old_state); + + if (!priv_state || !priv_state->no_vblank) drm_atomic_helper_wait_for_vblanks(dev, old_state);
drm_atomic_helper_cleanup_planes(dev, old_state);
On Fri, May 28, 2021 at 12:20:58AM +0100, Paul Cercueil wrote:
Not sure this applies to your hw, but drm_crtc_state.no_vblank exists. Might want to move to that instead of rolling your own. Or explain why you need your own here in your own private state. It does look quite a bit like you're just rolling your own version of this support that helpers gained meanwhile. -Daniel
Hi Daniel,
Le mar., juin 1 2021 at 17:48:12 +0200, Daniel Vetter daniel@ffwll.ch a écrit :
If I use drm_crtc_state->no_vblank, then I need a custom .atomic_commit_tail() that only calls drm_atomic_helper_wait_for_vblanks() when !no_vblank. That works, but I don't understand why drm_atomic_helper_commit_tail() doesn't do that by default, and makes me think I'm using it wrong.
Cheers, -Paul
On Thu, Jun 10, 2021 at 04:09:19PM +0100, Paul Cercueil wrote:
So the recommendation is to have your own commit_tail and use drm_atomic_helper_wait_for_flip_done(). But also if wait_for_vblanks dies on you, there's a driver bug: If vblanks arent available, then the drm_crtc_vblank_get should fail. If that's not the case then I guess some bigger issues to be fixed because userspace might also do a vblank wait (for timing the next frame), so that really needs to work correctly.
That's kinda why I put that wait_for_vblank in there by default, it forces drivers to be correct.
If you're wondering how that's done: This is why the driver ->enable_vblank callback can return an error code. So maybe the real fix here is in there, and everything else can stay as-is?
Another thing is that if you call drm_crtc_vblank_on/off correctly, this should also work out correctly - attempted vblank waits outside of when the vblank is running should fail.
Maybe something fell off here in this area because it's tricky, but the infrastructure should be here already. -Daniel
The IPU scaling information is computed in the plane's ".atomic_check" callback, and used in the ".atomic_update" callback. As such, it is state-specific, and should be moved to the private state structure.
Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/gpu/drm/ingenic/ingenic-ipu.c | 73 ++++++++++++++++++++------- 1 file changed, 54 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c index 007cd547b285..b85d9a7f53d3 100644 --- a/drivers/gpu/drm/ingenic/ingenic-ipu.c +++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c @@ -47,6 +47,8 @@ struct soc_info {
struct ingenic_ipu_private_state { struct drm_private_state base; + + unsigned int num_w, num_h, denom_w, denom_h; };
struct ingenic_ipu { @@ -58,8 +60,6 @@ struct ingenic_ipu { const struct soc_info *soc_info; bool clk_enabled;
- unsigned int num_w, num_h, denom_w, denom_h; - dma_addr_t addr_y, addr_u, addr_v;
struct drm_property *sharpness_prop; @@ -85,6 +85,30 @@ to_ingenic_ipu_priv_state(struct drm_private_state *state) return container_of(state, struct ingenic_ipu_private_state, base); }
+static struct ingenic_ipu_private_state * +ingenic_ipu_get_priv_state(struct ingenic_ipu *priv, struct drm_atomic_state *state) +{ + struct drm_private_state *priv_state; + + priv_state = drm_atomic_get_private_obj_state(state, &priv->private_obj); + if (IS_ERR(priv_state)) + return ERR_CAST(priv_state); + + return to_ingenic_ipu_priv_state(priv_state); +} + +static struct ingenic_ipu_private_state * +ingenic_ipu_get_new_priv_state(struct ingenic_ipu *priv, struct drm_atomic_state *state) +{ + struct drm_private_state *priv_state; + + priv_state = drm_atomic_get_new_private_obj_state(state, &priv->private_obj); + if (!priv_state) + return NULL; + + return to_ingenic_ipu_priv_state(priv_state); +} + /* * Apply conventional cubic convolution kernel. Both parameters * and return value are 15.16 signed fixed-point. @@ -305,11 +329,16 @@ static void ingenic_ipu_plane_atomic_update(struct drm_plane *plane, const struct drm_format_info *finfo; u32 ctrl, stride = 0, coef_index = 0, format = 0; bool needs_modeset, upscaling_w, upscaling_h; + struct ingenic_ipu_private_state *ipu_state; int err;
if (!newstate || !newstate->fb) return;
+ ipu_state = ingenic_ipu_get_new_priv_state(ipu, state); + if (WARN_ON(!ipu_state)) + return; + finfo = drm_format_info(newstate->fb->format->format);
if (!ipu->clk_enabled) { @@ -482,27 +511,27 @@ static void ingenic_ipu_plane_atomic_update(struct drm_plane *plane, if (ipu->soc_info->has_bicubic) ctrl |= JZ_IPU_CTRL_ZOOM_SEL;
- upscaling_w = ipu->num_w > ipu->denom_w; + upscaling_w = ipu_state->num_w > ipu_state->denom_w; if (upscaling_w) ctrl |= JZ_IPU_CTRL_HSCALE;
- if (ipu->num_w != 1 || ipu->denom_w != 1) { + if (ipu_state->num_w != 1 || ipu_state->denom_w != 1) { if (!ipu->soc_info->has_bicubic && !upscaling_w) - coef_index |= (ipu->denom_w - 1) << 16; + coef_index |= (ipu_state->denom_w - 1) << 16; else - coef_index |= (ipu->num_w - 1) << 16; + coef_index |= (ipu_state->num_w - 1) << 16; ctrl |= JZ_IPU_CTRL_HRSZ_EN; }
- upscaling_h = ipu->num_h > ipu->denom_h; + upscaling_h = ipu_state->num_h > ipu_state->denom_h; if (upscaling_h) ctrl |= JZ_IPU_CTRL_VSCALE;
- if (ipu->num_h != 1 || ipu->denom_h != 1) { + if (ipu_state->num_h != 1 || ipu_state->denom_h != 1) { if (!ipu->soc_info->has_bicubic && !upscaling_h) - coef_index |= ipu->denom_h - 1; + coef_index |= ipu_state->denom_h - 1; else - coef_index |= ipu->num_h - 1; + coef_index |= ipu_state->num_h - 1; ctrl |= JZ_IPU_CTRL_VRSZ_EN; }
@@ -513,13 +542,13 @@ static void ingenic_ipu_plane_atomic_update(struct drm_plane *plane, /* Set the LUT index register */ regmap_write(ipu->map, JZ_REG_IPU_RSZ_COEF_INDEX, coef_index);
- if (ipu->num_w != 1 || ipu->denom_w != 1) + if (ipu_state->num_w != 1 || ipu_state->denom_w != 1) ingenic_ipu_set_coefs(ipu, JZ_REG_IPU_HRSZ_COEF_LUT, - ipu->num_w, ipu->denom_w); + ipu_state->num_w, ipu_state->denom_w);
- if (ipu->num_h != 1 || ipu->denom_h != 1) + if (ipu_state->num_h != 1 || ipu_state->denom_h != 1) ingenic_ipu_set_coefs(ipu, JZ_REG_IPU_VRSZ_COEF_LUT, - ipu->num_h, ipu->denom_h); + ipu_state->num_h, ipu_state->denom_h);
/* Clear STATUS register */ regmap_write(ipu->map, JZ_REG_IPU_STATUS, 0); @@ -531,7 +560,8 @@ static void ingenic_ipu_plane_atomic_update(struct drm_plane *plane, dev_dbg(ipu->dev, "Scaling %ux%u to %ux%u (%u:%u horiz, %u:%u vert)\n", newstate->src_w >> 16, newstate->src_h >> 16, newstate->crtc_w, newstate->crtc_h, - ipu->num_w, ipu->denom_w, ipu->num_h, ipu->denom_h); + ipu_state->num_w, ipu_state->denom_w, + ipu_state->num_h, ipu_state->denom_h); }
static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane, @@ -545,6 +575,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane, struct ingenic_ipu *ipu = plane_to_ingenic_ipu(plane); struct drm_crtc *crtc = new_plane_state->crtc ?: old_plane_state->crtc; struct drm_crtc_state *crtc_state; + struct ingenic_ipu_private_state *ipu_state;
if (!crtc) return 0; @@ -553,6 +584,10 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane, if (WARN_ON(!crtc_state)) return -EINVAL;
+ ipu_state = ingenic_ipu_get_priv_state(ipu, state); + if (IS_ERR(ipu_state)) + return PTR_ERR(ipu_state); + /* Request a full modeset if we are enabling or disabling the IPU. */ if (!old_plane_state->crtc ^ !new_plane_state->crtc) crtc_state->mode_changed = true; @@ -605,10 +640,10 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane, if (num_h > max_h) return -EINVAL;
- ipu->num_w = num_w; - ipu->num_h = num_h; - ipu->denom_w = denom_w; - ipu->denom_h = denom_h; + ipu_state->num_w = num_w; + ipu_state->num_h = num_h; + ipu_state->denom_w = denom_w; + ipu_state->denom_h = denom_h;
out_check_damage: if (ingenic_drm_map_noncoherent(ipu->master))
Setting the DMA descriptor chain register in the probe function has been fine until now, because we only ever had one descriptor per foreground.
As the driver will soon have real descriptor chains, and the DMA descriptor chain register updates itself to point to the current descriptor being processed, this register needs to be reset after a full modeset to point to the first descriptor of the chain.
Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index 639994329c60..5ba3283da97d 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -210,6 +210,10 @@ static void ingenic_drm_crtc_atomic_enable(struct drm_crtc *crtc,
regmap_write(priv->map, JZ_REG_LCD_STATE, 0);
+ /* Set address of our DMA descriptor chain */ + regmap_write(priv->map, JZ_REG_LCD_DA0, dma_hwdesc_addr(priv, 0)); + regmap_write(priv->map, JZ_REG_LCD_DA1, dma_hwdesc_addr(priv, 1)); + regmap_update_bits(priv->map, JZ_REG_LCD_CTRL, JZ_LCD_CTRL_ENABLE | JZ_LCD_CTRL_DISABLE, JZ_LCD_CTRL_ENABLE); @@ -1218,10 +1222,6 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) } }
- /* Set address of our DMA descriptor chain */ - regmap_write(priv->map, JZ_REG_LCD_DA0, dma_hwdesc_phys_f0); - regmap_write(priv->map, JZ_REG_LCD_DA1, dma_hwdesc_phys_f1); - /* Enable OSD if available */ if (soc_info->has_osd) regmap_write(priv->map, JZ_REG_LCD_OSDC, JZ_LCD_OSDC_OSDEN);
When using C8 color mode, make sure that the palette is always uploaded before a frame; otherwise the very first frame will have wrong colors.
Do that by changing the link order of the DMA descriptors.
Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 45 ++++++++++++++++++----- 1 file changed, 35 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index 5ba3283da97d..ced2109e8f35 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -68,6 +68,7 @@ struct ingenic_drm_private_state { struct drm_private_state base;
bool no_vblank; + bool use_palette; };
struct ingenic_drm { @@ -185,6 +186,13 @@ static inline dma_addr_t dma_hwdesc_addr(const struct ingenic_drm *priv, bool us return priv->dma_hwdescs_phys + offset; }
+static inline dma_addr_t dma_hwdesc_pal_addr(const struct ingenic_drm *priv) +{ + u32 offset = offsetof(struct ingenic_dma_hwdescs, hwdesc_pal); + + return priv->dma_hwdescs_phys + offset; +} + static int ingenic_drm_update_pixclk(struct notifier_block *nb, unsigned long action, void *data) @@ -207,11 +215,19 @@ static void ingenic_drm_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *state) { struct ingenic_drm *priv = drm_crtc_get_priv(crtc); + struct ingenic_drm_private_state *priv_state; + + priv_state = ingenic_drm_get_new_priv_state(priv, state); + if (WARN_ON(!priv_state)) + return;
regmap_write(priv->map, JZ_REG_LCD_STATE, 0);
/* Set address of our DMA descriptor chain */ - regmap_write(priv->map, JZ_REG_LCD_DA0, dma_hwdesc_addr(priv, 0)); + if (priv_state->use_palette) + regmap_write(priv->map, JZ_REG_LCD_DA0, dma_hwdesc_pal_addr(priv)); + else + regmap_write(priv->map, JZ_REG_LCD_DA0, dma_hwdesc_addr(priv, 0)); regmap_write(priv->map, JZ_REG_LCD_DA1, dma_hwdesc_addr(priv, 1));
regmap_update_bits(priv->map, JZ_REG_LCD_CTRL, @@ -422,6 +438,7 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane, struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane); struct ingenic_drm *priv = drm_device_get_priv(plane->dev); + struct ingenic_drm_private_state *priv_state; struct drm_crtc_state *crtc_state; struct drm_crtc *crtc = new_plane_state->crtc ?: old_plane_state->crtc; int ret; @@ -434,6 +451,10 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane, if (WARN_ON(!crtc_state)) return -EINVAL;
+ priv_state = ingenic_drm_get_priv_state(priv, state); + if (IS_ERR(priv_state)) + return PTR_ERR(priv_state); + ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state, DRM_PLANE_HELPER_NO_SCALING, DRM_PLANE_HELPER_NO_SCALING, @@ -452,6 +473,9 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane, (new_plane_state->src_h >> 16) != new_plane_state->crtc_h)) return -EINVAL;
+ priv_state->use_palette = new_plane_state->fb && + new_plane_state->fb->format->format == DRM_FORMAT_C8; + /* * Require full modeset if enabling or disabling a plane, or changing * its position, size or depth. @@ -611,10 +635,11 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, struct ingenic_drm *priv = drm_device_get_priv(plane->dev); struct drm_plane_state *newstate = drm_atomic_get_new_plane_state(state, plane); struct drm_plane_state *oldstate = drm_atomic_get_old_plane_state(state, plane); + struct ingenic_drm_private_state *priv_state; struct drm_crtc_state *crtc_state; struct ingenic_dma_hwdesc *hwdesc; - unsigned int width, height, cpp, offset; - dma_addr_t addr; + unsigned int width, height, cpp; + dma_addr_t addr, next_addr; bool use_f1; u32 fourcc;
@@ -630,23 +655,23 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, height = newstate->src_h >> 16; cpp = newstate->fb->format->cpp[0];
+ priv_state = ingenic_drm_get_new_priv_state(priv, state); + if (priv_state && priv_state->use_palette) + next_addr = dma_hwdesc_pal_addr(priv); + else + next_addr = dma_hwdesc_addr(priv, use_f1); + hwdesc = &priv->dma_hwdescs->hwdesc[use_f1];
hwdesc->addr = addr; hwdesc->cmd = JZ_LCD_CMD_EOF_IRQ | (width * height * cpp / 4); + hwdesc->next = next_addr;
if (drm_atomic_crtc_needs_modeset(crtc_state)) { fourcc = newstate->fb->format->format;
ingenic_drm_plane_config(priv->dev, plane, fourcc);
- if (fourcc == DRM_FORMAT_C8) - offset = offsetof(struct ingenic_dma_hwdescs, hwdesc_pal); - else - offset = offsetof(struct ingenic_dma_hwdescs, hwdesc[0]); - - priv->dma_hwdescs->hwdesc[0].next = priv->dma_hwdescs_phys + offset; - crtc_state->color_mgmt_changed = fourcc == DRM_FORMAT_C8; }
Add boilerplate code to support a custom "ingenic_gem_object". Empty for now, but it will be useful later when subsequent patches will introduce object-specific driver data.
Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index ced2109e8f35..1cac369f6293 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -64,6 +64,10 @@ struct jz_soc_info { unsigned int num_formats_f0, num_formats_f1; };
+struct ingenic_gem_object { + struct drm_gem_cma_object base; +}; + struct ingenic_drm_private_state { struct drm_private_state base;
@@ -179,6 +183,11 @@ static inline struct ingenic_drm *drm_nb_get_priv(struct notifier_block *nb) return container_of(nb, struct ingenic_drm, clock_nb); }
+static inline struct ingenic_gem_object *to_ingenic_gem_obj(struct drm_gem_object *gem_obj) +{ + return container_of(gem_obj, struct ingenic_gem_object, base.base); +} + static inline dma_addr_t dma_hwdesc_addr(const struct ingenic_drm *priv, bool use_f1) { u32 offset = offsetof(struct ingenic_dma_hwdescs, hwdesc[use_f1]); @@ -853,15 +862,15 @@ static struct drm_gem_object * ingenic_drm_gem_create_object(struct drm_device *drm, size_t size) { struct ingenic_drm *priv = drm_device_get_priv(drm); - struct drm_gem_cma_object *obj; + struct ingenic_gem_object *obj;
obj = kzalloc(sizeof(*obj), GFP_KERNEL); if (!obj) return ERR_PTR(-ENOMEM);
- obj->map_noncoherent = priv->soc_info->map_noncoherent; + obj->base.map_noncoherent = priv->soc_info->map_noncoherent;
- return &obj->base; + return &obj->base.base; }
static struct drm_private_state *
Add a ingenic_drm_gem_fb_destroy() function, which currently only calls gem_fb_destroy(), but will be extended in a subsequent patch.
Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 26 +++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index 1cac369f6293..2761478b16e8 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -846,16 +846,38 @@ static void ingenic_drm_disable_vblank(struct drm_crtc *crtc) regmap_update_bits(priv->map, JZ_REG_LCD_CTRL, JZ_LCD_CTRL_EOF_IRQ, 0); }
+static void ingenic_drm_gem_fb_destroy(struct drm_framebuffer *fb) +{ + drm_gem_fb_destroy(fb); +} + +static const struct drm_framebuffer_funcs ingenic_drm_gem_fb_funcs = { + .destroy = ingenic_drm_gem_fb_destroy, + .create_handle = drm_gem_fb_create_handle, +}; + +static const struct drm_framebuffer_funcs ingenic_drm_gem_fb_funcs_dirty = { + .destroy = ingenic_drm_gem_fb_destroy, + .create_handle = drm_gem_fb_create_handle, + .dirty = drm_atomic_helper_dirtyfb, +}; + static struct drm_framebuffer * ingenic_drm_gem_fb_create(struct drm_device *drm, struct drm_file *file, const struct drm_mode_fb_cmd2 *mode_cmd) { struct ingenic_drm *priv = drm_device_get_priv(drm); + const struct drm_framebuffer_funcs *fb_funcs; + struct drm_framebuffer *fb;
if (priv->soc_info->map_noncoherent) - return drm_gem_fb_create_with_dirty(drm, file, mode_cmd); + fb_funcs = &ingenic_drm_gem_fb_funcs_dirty; + else + fb_funcs = &ingenic_drm_gem_fb_funcs; + + fb = drm_gem_fb_create_with_funcs(drm, file, mode_cmd, fb_funcs);
- return drm_gem_fb_create(drm, file, mode_cmd); + return fb; }
static struct drm_gem_object *
A lot of devices with an Ingenic SoC have a weird LCD panel attached, where the pixels are not square. For instance, the AUO A030JTN01 and Innolux EJ030NA panels have a resolution of 320x480 with a 4:3 aspect ratio.
All userspace applications are built with the assumption that the pixels are square. To be able to support these devices without too much effort, add a doublescan feature, which allows the f0 and f1 planes to be used with only half of the screen's vertical resolution, where each line of the input is displayed twice.
This is done using a chained list of DMA descriptors, one descriptor per output line.
Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 93 +++++++++++++++++++++-- 1 file changed, 87 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index 2761478b16e8..01d8490393d1 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -66,6 +66,8 @@ struct jz_soc_info {
struct ingenic_gem_object { struct drm_gem_cma_object base; + struct ingenic_dma_hwdesc *hwdescs; + dma_addr_t hwdescs_phys; };
struct ingenic_drm_private_state { @@ -73,6 +75,23 @@ struct ingenic_drm_private_state {
bool no_vblank; bool use_palette; + + /* + * A lot of devices with an Ingenic SoC have a weird LCD panel attached, + * where the pixels are not square. For instance, the AUO A030JTN01 and + * Innolux EJ030NA panels have a resolution of 320x480 with a 4:3 aspect + * ratio. + * + * All userspace applications are built with the assumption that the + * pixels are square. To be able to support these devices without too + * much effort, add a doublescan feature, which allows the f0 and f1 + * planes to be used with only half of the screen's vertical resolution, + * where each line of the input is displayed twice. + * + * This is done using a chained list of DMA descriptors, one descriptor + * per output line. + */ + bool doublescan; };
struct ingenic_drm { @@ -465,7 +484,7 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane, return PTR_ERR(priv_state);
ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state, - DRM_PLANE_HELPER_NO_SCALING, + 0x8000, DRM_PLANE_HELPER_NO_SCALING, priv->soc_info->has_osd, true); @@ -482,6 +501,17 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane, (new_plane_state->src_h >> 16) != new_plane_state->crtc_h)) return -EINVAL;
+ /* Enable doublescan if the CRTC_H is twice the SRC_H. */ + priv_state->doublescan = (new_plane_state->src_h >> 16) * 2 == new_plane_state->crtc_h; + + /* Otherwise, fail if CRTC_H != SRC_H */ + if (!priv_state->doublescan && (new_plane_state->src_h >> 16) != new_plane_state->crtc_h) + return -EINVAL; + + /* Fail if CRTC_W != SRC_W */ + if ((new_plane_state->src_w >> 16) != new_plane_state->crtc_w) + return -EINVAL; + priv_state->use_palette = new_plane_state->fb && new_plane_state->fb->format->format == DRM_FORMAT_C8;
@@ -647,7 +677,9 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, struct ingenic_drm_private_state *priv_state; struct drm_crtc_state *crtc_state; struct ingenic_dma_hwdesc *hwdesc; - unsigned int width, height, cpp; + unsigned int width, height, cpp, i; + struct drm_gem_object *gem_obj; + struct ingenic_gem_object *obj; dma_addr_t addr, next_addr; bool use_f1; u32 fourcc; @@ -664,17 +696,39 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane, height = newstate->src_h >> 16; cpp = newstate->fb->format->cpp[0];
+ gem_obj = drm_gem_fb_get_obj(newstate->fb, 0); + obj = to_ingenic_gem_obj(gem_obj); + priv_state = ingenic_drm_get_new_priv_state(priv, state); if (priv_state && priv_state->use_palette) next_addr = dma_hwdesc_pal_addr(priv); else next_addr = dma_hwdesc_addr(priv, use_f1);
- hwdesc = &priv->dma_hwdescs->hwdesc[use_f1]; + if (priv_state->doublescan) { + hwdesc = &obj->hwdescs[0]; + /* + * Use one DMA descriptor per output line, and display + * each input line twice. + */ + for (i = 0; i < newstate->crtc_h; i++) { + hwdesc[i].next = obj->hwdescs_phys + + (i + 1) * sizeof(*hwdesc); + hwdesc[i].addr = addr + (i / 2) * newstate->fb->pitches[0]; + hwdesc[i].cmd = newstate->fb->pitches[0] / 4; + }
- hwdesc->addr = addr; - hwdesc->cmd = JZ_LCD_CMD_EOF_IRQ | (width * height * cpp / 4); - hwdesc->next = next_addr; + /* We want the EOF IRQ only on the very last transfer */ + hwdesc[newstate->crtc_h - 1].cmd |= JZ_LCD_CMD_EOF_IRQ; + hwdesc[newstate->crtc_h - 1].next = next_addr; + priv->dma_hwdescs->hwdesc[use_f1] = *hwdesc; + } else { + /* Use one DMA descriptor for the whole frame. */ + hwdesc = &priv->dma_hwdescs->hwdesc[use_f1]; + hwdesc->addr = addr; + hwdesc->cmd = JZ_LCD_CMD_EOF_IRQ | (width * height * cpp / 4); + hwdesc->next = next_addr; + }
if (drm_atomic_crtc_needs_modeset(crtc_state)) { fourcc = newstate->fb->format->format; @@ -848,6 +902,13 @@ static void ingenic_drm_disable_vblank(struct drm_crtc *crtc)
static void ingenic_drm_gem_fb_destroy(struct drm_framebuffer *fb) { + struct ingenic_drm *priv = drm_device_get_priv(fb->dev); + struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0); + struct ingenic_gem_object *obj = to_ingenic_gem_obj(gem_obj); + + dma_free_coherent(priv->dev, + sizeof(*obj->hwdescs) * fb->height, + obj->hwdescs, obj->hwdescs_phys); drm_gem_fb_destroy(fb); }
@@ -868,6 +929,8 @@ ingenic_drm_gem_fb_create(struct drm_device *drm, struct drm_file *file, { struct ingenic_drm *priv = drm_device_get_priv(drm); const struct drm_framebuffer_funcs *fb_funcs; + struct drm_gem_object *gem_obj; + struct ingenic_gem_object *obj; struct drm_framebuffer *fb;
if (priv->soc_info->map_noncoherent) @@ -876,6 +939,24 @@ ingenic_drm_gem_fb_create(struct drm_device *drm, struct drm_file *file, fb_funcs = &ingenic_drm_gem_fb_funcs;
fb = drm_gem_fb_create_with_funcs(drm, file, mode_cmd, fb_funcs); + if (IS_ERR(fb)) + return fb; + + gem_obj = drm_gem_fb_get_obj(fb, 0); + obj = to_ingenic_gem_obj(gem_obj); + + /* + * Create (fb->height * 2) DMA descriptors, in case we want to use the + * doublescan feature. + */ + obj->hwdescs = dma_alloc_coherent(priv->dev, + sizeof(*obj->hwdescs) * fb->height * 2, + &obj->hwdescs_phys, + GFP_KERNEL); + if (!obj->hwdescs) { + drm_gem_fb_destroy(fb); + return ERR_PTR(-ENOMEM); + }
return fb; }
Attach a top-level bridge to each encoder, which will be used for negociating the bus format and flags.
Signed-off-by: Paul Cercueil paul@crapouillou.net --- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 98 ++++++++++++++++++----- 1 file changed, 77 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index 01d8490393d1..f0242e917d6e 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -21,6 +21,7 @@ #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_bridge.h> +#include <drm/drm_bridge_connector.h> #include <drm/drm_color_mgmt.h> #include <drm/drm_crtc.h> #include <drm/drm_crtc_helper.h> @@ -132,6 +133,26 @@ struct ingenic_drm { struct drm_private_obj private_obj; };
+struct ingenic_drm_bridge { + struct drm_encoder encoder; + struct drm_bridge bridge; + struct drm_bridge *next_bridge; + + /* + * FIXME: this should really be in ingenic_drm_private_state, but there + * doesn't seem to be a way to retrieve a pointer to it from within + * ingenic_drm_encoder_atomic_mode_set (no drm_atomic_state + * back-pointers). + */ + struct drm_bus_cfg bus_cfg; +}; + +static inline struct ingenic_drm_bridge * +to_ingenic_drm_bridge(struct drm_encoder *encoder) +{ + return container_of(encoder, struct ingenic_drm_bridge, encoder); +} + static inline struct ingenic_drm_private_state * to_ingenic_drm_priv_state(struct drm_private_state *state) { @@ -749,11 +770,10 @@ static void ingenic_drm_encoder_atomic_mode_set(struct drm_encoder *encoder, { struct ingenic_drm *priv = drm_device_get_priv(encoder->dev); struct drm_display_mode *mode = &crtc_state->adjusted_mode; - struct drm_connector *conn = conn_state->connector; - struct drm_display_info *info = &conn->display_info; + struct ingenic_drm_bridge *bridge = to_ingenic_drm_bridge(encoder); unsigned int cfg, rgbcfg = 0;
- priv->panel_is_sharp = info->bus_flags & DRM_BUS_FLAG_SHARP_SIGNALS; + priv->panel_is_sharp = bridge->bus_cfg.flags & DRM_BUS_FLAG_SHARP_SIGNALS;
if (priv->panel_is_sharp) { cfg = JZ_LCD_CFG_MODE_SPECIAL_TFT_1 | JZ_LCD_CFG_REV_POLARITY; @@ -766,19 +786,19 @@ static void ingenic_drm_encoder_atomic_mode_set(struct drm_encoder *encoder, cfg |= JZ_LCD_CFG_HSYNC_ACTIVE_LOW; if (mode->flags & DRM_MODE_FLAG_NVSYNC) cfg |= JZ_LCD_CFG_VSYNC_ACTIVE_LOW; - if (info->bus_flags & DRM_BUS_FLAG_DE_LOW) + if (bridge->bus_cfg.flags & DRM_BUS_FLAG_DE_LOW) cfg |= JZ_LCD_CFG_DE_ACTIVE_LOW; - if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE) + if (bridge->bus_cfg.flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE) cfg |= JZ_LCD_CFG_PCLK_FALLING_EDGE;
if (!priv->panel_is_sharp) { - if (conn->connector_type == DRM_MODE_CONNECTOR_TV) { + if (conn_state->connector->connector_type == DRM_MODE_CONNECTOR_TV) { if (mode->flags & DRM_MODE_FLAG_INTERLACE) cfg |= JZ_LCD_CFG_MODE_TV_OUT_I; else cfg |= JZ_LCD_CFG_MODE_TV_OUT_P; } else { - switch (*info->bus_formats) { + switch (bridge->bus_cfg.format) { case MEDIA_BUS_FMT_RGB565_1X16: cfg |= JZ_LCD_CFG_MODE_GENERIC_16BIT; break; @@ -804,20 +824,31 @@ static void ingenic_drm_encoder_atomic_mode_set(struct drm_encoder *encoder, regmap_write(priv->map, JZ_REG_LCD_RGBC, rgbcfg); }
-static int ingenic_drm_encoder_atomic_check(struct drm_encoder *encoder, - struct drm_crtc_state *crtc_state, - struct drm_connector_state *conn_state) +static int ingenic_drm_bridge_attach(struct drm_bridge *bridge, + enum drm_bridge_attach_flags flags) +{ + struct drm_encoder *encoder = bridge->encoder; + struct ingenic_drm_bridge *ingenic_bridge = to_ingenic_drm_bridge(encoder); + + return drm_bridge_attach(encoder, ingenic_bridge->next_bridge, + &ingenic_bridge->bridge, flags); +} + +static int ingenic_drm_bridge_atomic_check(struct drm_bridge *bridge, + struct drm_bridge_state *bridge_state, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) { - struct drm_display_info *info = &conn_state->connector->display_info; struct drm_display_mode *mode = &crtc_state->adjusted_mode; + struct drm_encoder *encoder = bridge->encoder; + struct ingenic_drm_bridge *ingenic_bridge = to_ingenic_drm_bridge(encoder);
- if (info->num_bus_formats != 1) - return -EINVAL; + ingenic_bridge->bus_cfg = bridge_state->output_bus_cfg;
if (conn_state->connector->connector_type == DRM_MODE_CONNECTOR_TV) return 0;
- switch (*info->bus_formats) { + switch (bridge_state->output_bus_cfg.format) { case MEDIA_BUS_FMT_RGB888_3X8: case MEDIA_BUS_FMT_RGB888_3X8_DELTA: /* @@ -1056,8 +1087,16 @@ static const struct drm_crtc_helper_funcs ingenic_drm_crtc_helper_funcs = { };
static const struct drm_encoder_helper_funcs ingenic_drm_encoder_helper_funcs = { - .atomic_mode_set = ingenic_drm_encoder_atomic_mode_set, - .atomic_check = ingenic_drm_encoder_atomic_check, + .atomic_mode_set = ingenic_drm_encoder_atomic_mode_set, +}; + +static const struct drm_bridge_funcs ingenic_drm_bridge_funcs = { + .attach = ingenic_drm_bridge_attach, + .atomic_check = ingenic_drm_bridge_atomic_check, + .atomic_reset = drm_atomic_helper_bridge_reset, + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, + .atomic_get_input_bus_fmts = drm_atomic_helper_bridge_propagate_bus_fmt, };
static const struct drm_mode_config_funcs ingenic_drm_mode_config_funcs = { @@ -1097,12 +1136,14 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) { struct platform_device *pdev = to_platform_device(dev); struct ingenic_drm_private_state *private_state; + struct ingenic_drm_bridge *ingenic_bridge; 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_connector *connector; struct drm_encoder *encoder; struct drm_device *drm; void __iomem *base; @@ -1291,22 +1332,37 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) bridge = devm_drm_panel_bridge_add_typed(dev, panel, DRM_MODE_CONNECTOR_DPI);
- encoder = drmm_plain_encoder_alloc(drm, NULL, DRM_MODE_ENCODER_DPI, NULL); - if (IS_ERR(encoder)) { - ret = PTR_ERR(encoder); + ingenic_bridge = drmm_encoder_alloc(drm, struct ingenic_drm_bridge, + encoder, NULL, + DRM_MODE_ENCODER_DPI, NULL); + if (IS_ERR(ingenic_bridge)) { + ret = PTR_ERR(ingenic_bridge); dev_err(dev, "Failed to init encoder: %d\n", ret); return ret; }
- encoder->possible_crtcs = 1; + encoder = &ingenic_bridge->encoder; + encoder->possible_crtcs = drm_crtc_mask(&priv->crtc);
drm_encoder_helper_add(encoder, &ingenic_drm_encoder_helper_funcs);
- ret = drm_bridge_attach(encoder, bridge, NULL, 0); + ingenic_bridge->bridge.funcs = &ingenic_drm_bridge_funcs; + ingenic_bridge->next_bridge = bridge; + + ret = drm_bridge_attach(encoder, &ingenic_bridge->bridge, NULL, + DRM_BRIDGE_ATTACH_NO_CONNECTOR); if (ret) { dev_err(dev, "Unable to attach bridge\n"); return ret; } + + connector = drm_bridge_connector_init(drm, encoder); + if (IS_ERR(connector)) { + dev_err(dev, "Unable to init connector\n"); + return PTR_ERR(connector); + } + + drm_connector_attach_encoder(connector, encoder); }
drm_for_each_encoder(encoder, drm) {
On Fri, May 28, 2021 at 12:22:05AM +0100, Paul Cercueil wrote:
You can have multiple fb pointing at the same gem bo. I think with that this blows up pretty badly.
I think right call is to just move the doublescan mapping to the fb object entirely, and not have a subclassed gem bo. Userspace shouldn't recreate fb objects just for fun, so caching at the gem bo level shouldn't be needed. But if you do need it then you need to refcount it so multiple fb objects all work out (and they can have different modes even used for them).
Another option is attaching this to the drm_plane state and setting it up in prepare_plane/cleanup_plane. But then you probably need some caching at the obj level (but a simple single slot/last hit cache should be good enough here). I think this would be the cleanest since doing very expensive things attached to fb might cause issues, userspace can create/destroy quite a few of them as part of atomic_test (which skips the prepare/cleanup_plane part). So this would be the right option if setting up this dma mapping for doublescan is a more expensive thing to do. -Daniel
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Paul Cercueil