This patch series refactors atomic commit tail handling in the R-Car DU driver to simplify the code flow, and open the door to further optimisations. This iteration rebases the previous work and fixes a bug reported by the build bots.
The R-Car DU is a bit of a strange beast, with support for up to four CRTCs that share resources in groups of two CRTCs. Depending on the generation, planes can be shared (on Gen 1 and Gen 2), and output routing configuration is also handled at the group level to some extent. Furthermore, many configuration parameters, especially those related to routing or clock handling, require the whole group to be restarted to take effect, even when the parameter itself affects a single CRTC only.
This hardware architecture is difficult to handle properly on the software side, and has resulted in group usage being reference-counted while CRTC usage only tracks the enabled state. Calls are then unbalanced and difficult to trace, especially for the configuration of output routing, and implementation of new shared resources is hindered. This patch series aims at solving this problem.
The series starts with 4 patches that touch the API between the DU and VSP drivers. It became apparent that we need to split the configuration of the VSP to allow fine grain control of setting the mode configuration and enabling/disabling of the pipeline. To support the cross-component API, the new interface is added in patch 01/10, including an implementation of vsp1_du_setup_lif() to support the transition. Patch 02/10 prepares for the new call flow that will call the atomic flush handler before enabling the pipeline. The DRM usage is adapted in patch 03/10, before the call is removed entirely in patch 04/10.
The next two patches convert CRTC clock handling and initial setup, potentially called from both the CRTC .atomic_begin() and .atomic_enable() operations, to a simpler code flow controlled by the commit tail handler. Patch 05/10 takes the CRTCs out of standby and put them back in standby respectively at the beginning and end of the commit tail handler, based on the CRTC atomic state instead of state information stored in the custom rcar_du_crtc structure. Patch 06/10 then performs a similar change for the CRTC mode setting configuration.
Finally, the last four patches introduce a DRM private object for the CRTC groups, along with an associated state. Patch 07/10 adds a helper macro to easily iterate over CRTC groups, and patch 08/10 adds the group private objects and empty states. Patches 09/10 and 10/10 respectively move the group setup and routing configuration under control of the commit tail handler, simplifying the configuration and moving state information from driver structures to state structures.
More refactoring is expected, with plane assignment being moved to group states, and group restart being optimised to avoid flickering. Better configuration of pixel clocks could also be implemented on top of this series.
The whole series has been tested on Salvator-XS with the DU test suite (http://git.ideasonboard.com/renesas/kms-tests.git). No failure or change in behaviour has been noticed.
Kieran Bingham (8): media: vsp1: drm: Split vsp1_du_setup_lif() drm: rcar-du: Convert to the new VSP atomic API media: vsp1: drm: Remove vsp1_du_setup_lif() drm: rcar-du: Handle CRTC standby from commit tail handler drm: rcar-du: Handle CRTC configuration from commit tail handler drm: rcar-du: Provide for_each_group helper drm: rcar-du: Create a group state object drm: rcar-du: Perform group setup from the atomic tail handler
Laurent Pinchart (2): media: vsp1: drm: Don't configure hardware when the pipeline is disabled drm: rcar-du: Centralise routing configuration in commit tail handler
drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 160 ++++++---- drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 9 +- drivers/gpu/drm/rcar-du/rcar_du_drv.h | 6 +- drivers/gpu/drm/rcar-du/rcar_du_group.c | 390 +++++++++++++++++++----- drivers/gpu/drm/rcar-du/rcar_du_group.h | 44 ++- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 63 ++-- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 10 +- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 20 +- drivers/gpu/drm/rcar-du/rcar_du_vsp.h | 3 + drivers/media/platform/vsp1/vsp1_drm.c | 188 ++++++++---- drivers/media/platform/vsp1/vsp1_drm.h | 2 + include/media/vsp1.h | 25 +- 12 files changed, 644 insertions(+), 276 deletions(-)
Break vsp1_du_setup_lif() into components more suited to the DRM Atomic API. The existing vsp1_du_setup_lif() API call is maintained as it is still used from the DU.
Reviewed-by: Ulrich Hecht uli+renesas@fpond.eu Signed-off-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- Changes since v3: - Minor formatting updates to fix checkpatch.pl --strict - Spelling correction.
Changes since v2:
- Minor formatting changes - Fix NULL pointer dereference in vsp1_du_setup_lif() --- drivers/media/platform/vsp1/vsp1_drm.c | 219 ++++++++++++++++++------- include/media/vsp1.h | 31 +++- 2 files changed, 187 insertions(+), 63 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index 06f74d410973..b2c98229c0f1 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -616,10 +616,10 @@ int vsp1_du_init(struct device *dev) EXPORT_SYMBOL_GPL(vsp1_du_init);
/** - * vsp1_du_setup_lif - Setup the output part of the VSP pipeline + * vsp1_du_atomic_modeset - Configure the mode as part of an atomic update * @dev: the VSP device * @pipe_index: the DRM pipeline index - * @cfg: the LIF configuration + * @cfg: the mode configuration * * Configure the output part of VSP DRM pipeline for the given frame @cfg.width * and @cfg.height. This sets up formats on the BRx source pad, the WPF sink and @@ -636,14 +636,12 @@ EXPORT_SYMBOL_GPL(vsp1_du_init); * * Return 0 on success or a negative error code on failure. */ -int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, - const struct vsp1_du_lif_config *cfg) +int vsp1_du_atomic_modeset(struct device *dev, unsigned int pipe_index, + const struct vsp1_du_modeset_config *cfg) { struct vsp1_device *vsp1 = dev_get_drvdata(dev); struct vsp1_drm_pipeline *drm_pipe; struct vsp1_pipeline *pipe; - unsigned long flags; - unsigned int i; int ret;
if (pipe_index >= vsp1->info->lif_count) @@ -652,60 +650,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, drm_pipe = &vsp1->drm->pipe[pipe_index]; pipe = &drm_pipe->pipe;
- if (!cfg) { - struct vsp1_brx *brx; - - mutex_lock(&vsp1->drm->lock); - - brx = to_brx(&pipe->brx->subdev); - - /* - * NULL configuration means the CRTC is being disabled, stop - * the pipeline and turn the light off. - */ - ret = vsp1_pipeline_stop(pipe); - if (ret == -ETIMEDOUT) - dev_err(vsp1->dev, "DRM pipeline stop timeout\n"); - - for (i = 0; i < ARRAY_SIZE(pipe->inputs); ++i) { - struct vsp1_rwpf *rpf = pipe->inputs[i]; - - if (!rpf) - continue; - - /* - * Remove the RPF from the pipe and the list of BRx - * inputs. - */ - WARN_ON(!rpf->entity.pipe); - rpf->entity.pipe = NULL; - list_del(&rpf->entity.list_pipe); - pipe->inputs[i] = NULL; - - brx->inputs[rpf->brx_input].rpf = NULL; - } - - drm_pipe->du_complete = NULL; - pipe->num_inputs = 0; - - dev_dbg(vsp1->dev, "%s: pipe %u: releasing %s\n", - __func__, pipe->lif->index, - BRX_NAME(pipe->brx)); - - list_del(&pipe->brx->list_pipe); - pipe->brx->pipe = NULL; - pipe->brx = NULL; - - mutex_unlock(&vsp1->drm->lock); - - vsp1_dlm_reset(pipe->output->dlm); - vsp1_device_put(vsp1); - - dev_dbg(vsp1->dev, "%s: pipeline disabled\n", __func__); - - return 0; - } - drm_pipe->width = cfg->width; drm_pipe->height = cfg->height; pipe->interlaced = cfg->interlaced; @@ -722,8 +666,43 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, goto unlock;
ret = vsp1_du_pipeline_setup_output(vsp1, pipe); - if (ret < 0) - goto unlock; + +unlock: + mutex_unlock(&vsp1->drm->lock); + + return ret; +} + +/** + * vsp1_du_atomic_enable - Enable and start a DU pipeline + * @dev: the VSP device + * @pipe_index: the DRM pipeline index + * @cfg: the enablement configuration + * + * The @pipe_index argument selects which DRM pipeline to enable. The number of + * available pipelines depends on the VSP instance. + * + * The configuration passes a callback function to register notification of + * frame completion events. + * + * Return 0 on success or a negative error code on failure. + */ +int vsp1_du_atomic_enable(struct device *dev, unsigned int pipe_index, + const struct vsp1_du_enable_config *cfg) +{ + struct vsp1_device *vsp1 = dev_get_drvdata(dev); + struct vsp1_drm_pipeline *drm_pipe; + struct vsp1_pipeline *pipe; + unsigned long flags; + int ret; + + if (pipe_index >= vsp1->info->lif_count) + return -EINVAL; + + drm_pipe = &vsp1->drm->pipe[pipe_index]; + pipe = &drm_pipe->pipe; + + mutex_lock(&vsp1->drm->lock);
/* Enable the VSP1. */ ret = vsp1_device_get(vsp1); @@ -759,6 +738,122 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
return 0; } +EXPORT_SYMBOL_GPL(vsp1_du_atomic_enable); + +/** + * vsp1_du_atomic_disable - Disable and stop a DU pipeline + * @dev: the VSP device + * @pipe_index: the DRM pipeline index + * + * The @pipe_index argument selects which DRM pipeline to disable. The number + * of available pipelines depend on the VSP instance. + * + * Return 0 on success or a negative error code on failure. + */ +int vsp1_du_atomic_disable(struct device *dev, unsigned int pipe_index) +{ + struct vsp1_device *vsp1 = dev_get_drvdata(dev); + struct vsp1_drm_pipeline *drm_pipe; + struct vsp1_pipeline *pipe; + struct vsp1_brx *brx; + unsigned int i; + int ret; + + if (pipe_index >= vsp1->info->lif_count) + return -EINVAL; + + drm_pipe = &vsp1->drm->pipe[pipe_index]; + pipe = &drm_pipe->pipe; + + mutex_lock(&vsp1->drm->lock); + + brx = to_brx(&pipe->brx->subdev); + + /* Stop the pipeline and turn the light off. */ + ret = vsp1_pipeline_stop(pipe); + if (ret == -ETIMEDOUT) + dev_err(vsp1->dev, "DRM pipeline stop timeout\n"); + + for (i = 0; i < ARRAY_SIZE(pipe->inputs); ++i) { + struct vsp1_rwpf *rpf = pipe->inputs[i]; + + if (!rpf) + continue; + + /* Remove the RPF from the pipe and the list of BRx inputs. */ + WARN_ON(!rpf->entity.pipe); + rpf->entity.pipe = NULL; + list_del(&rpf->entity.list_pipe); + pipe->inputs[i] = NULL; + + brx->inputs[rpf->brx_input].rpf = NULL; + } + + drm_pipe->du_complete = NULL; + pipe->num_inputs = 0; + + dev_dbg(vsp1->dev, "%s: pipe %u: releasing %s\n", __func__, + pipe->lif->index, BRX_NAME(pipe->brx)); + + list_del(&pipe->brx->list_pipe); + pipe->brx->pipe = NULL; + pipe->brx = NULL; + + mutex_unlock(&vsp1->drm->lock); + + vsp1_dlm_reset(pipe->output->dlm); + vsp1_device_put(vsp1); + + dev_dbg(vsp1->dev, "%s: pipeline disabled\n", __func__); + + return 0; +} +EXPORT_SYMBOL_GPL(vsp1_du_atomic_disable); + +/** + * vsp1_du_setup_lif - Setup the output part of the VSP pipeline + * @dev: the VSP device + * @pipe_index: the DRM pipeline index + * @cfg: the LIF configuration + * + * Configure the output part of VSP DRM pipeline for the given frame @cfg.width + * and @cfg.height. This sets up formats on the BRx source pad, the WPF sink and + * source pads, and the LIF sink pad. + * + * The @pipe_index argument selects which DRM pipeline to setup. The number of + * available pipelines depend on the VSP instance. + * + * As the media bus code on the blend unit source pad is conditioned by the + * configuration of its sink 0 pad, we also set up the formats on all blend unit + * sinks, even if the configuration will be overwritten later by + * vsp1_du_setup_rpf(). This ensures that the blend unit configuration is set to + * a well defined state. + * + * Return 0 on success or a negative error code on failure. + */ +int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, + const struct vsp1_du_lif_config *cfg) +{ + struct vsp1_du_modeset_config modes; + struct vsp1_du_enable_config enable; + int ret; + + if (!cfg) + return vsp1_du_atomic_disable(dev, pipe_index); + + modes.width = cfg->width; + modes.height = cfg->height; + modes.interlaced = cfg->interlaced; + + ret = vsp1_du_atomic_modeset(dev, pipe_index, &modes); + if (ret) + return ret; + + enable.callback = cfg->callback; + enable.callback_data = cfg->callback_data; + + return vsp1_du_atomic_enable(dev, pipe_index, &enable); +} EXPORT_SYMBOL_GPL(vsp1_du_setup_lif);
/** diff --git a/include/media/vsp1.h b/include/media/vsp1.h index cc1b0d42ce95..253db8029752 100644 --- a/include/media/vsp1.h +++ b/include/media/vsp1.h @@ -21,7 +21,7 @@ int vsp1_du_init(struct device *dev); #define VSP1_DU_STATUS_WRITEBACK BIT(1)
/** - * struct vsp1_du_lif_config - VSP LIF configuration + * struct vsp1_du_lif_config - VSP LIF configuration - Deprecated * @width: output frame width * @height: output frame height * @interlaced: true for interlaced pipelines @@ -42,6 +42,30 @@ struct vsp1_du_lif_config { int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, const struct vsp1_du_lif_config *cfg);
+/** + * struct vsp1_du_modeset_config - VSP display mode configuration + * @width: output frame width + * @height: output frame height + * @interlaced: true for interlaced pipelines + */ +struct vsp1_du_modeset_config { + unsigned int width; + unsigned int height; + bool interlaced; +}; + +/** + * struct vsp1_du_enable_config - VSP enable configuration + * @callback: frame completion callback function (optional). When a callback + * is provided, the VSP driver guarantees that it will be called once + * and only once for each vsp1_du_atomic_flush() call. + * @callback_data: data to be passed to the frame completion callback + */ +struct vsp1_du_enable_config { + void (*callback)(void *data, unsigned int status, u32 crc); + void *callback_data; +}; + /** * struct vsp1_du_atomic_config - VSP atomic configuration parameters * @pixelformat: plane pixel format (V4L2 4CC) @@ -106,6 +130,11 @@ struct vsp1_du_atomic_pipe_config { struct vsp1_du_writeback_config writeback; };
+int vsp1_du_atomic_modeset(struct device *dev, unsigned int pipe_index, + const struct vsp1_du_modeset_config *cfg); +int vsp1_du_atomic_enable(struct device *dev, unsigned int pipe_index, + const struct vsp1_du_enable_config *cfg); +int vsp1_du_atomic_disable(struct device *dev, unsigned int pipe_index); void vsp1_du_atomic_begin(struct device *dev, unsigned int pipe_index); int vsp1_du_atomic_update(struct device *dev, unsigned int pipe_index, unsigned int rpf,
From: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
The vsp1_du_atomic_flush() function calls vsp1_du_pipeline_configure() to configure the hardware pipeline. The function is currently guaranteed to be called with the pipeline enabled, but this will change by future rework of the DU driver. Guard the hardware configuration to skip it when the pipeline is disabled. The hardware will be configured the next time the pipeline gets enabled.
Reviewed-by: Ulrich Hecht uli+renesas@fpond.eu Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Signed-off-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com --- drivers/media/platform/vsp1/vsp1_drm.c | 13 ++++++++++++- drivers/media/platform/vsp1/vsp1_drm.h | 2 ++ 2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index b2c98229c0f1..4132027ead6c 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -723,6 +723,8 @@ int vsp1_du_atomic_enable(struct device *dev, unsigned int pipe_index, /* Configure all entities in the pipeline. */ vsp1_du_pipeline_configure(pipe);
+ drm_pipe->enabled = true; + unlock: mutex_unlock(&vsp1->drm->lock);
@@ -799,6 +801,8 @@ int vsp1_du_atomic_disable(struct device *dev, unsigned int pipe_index) pipe->brx->pipe = NULL; pipe->brx = NULL;
+ drm_pipe->enabled = false; + mutex_unlock(&vsp1->drm->lock);
vsp1_dlm_reset(pipe->output->dlm); @@ -991,7 +995,14 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index, }
vsp1_du_pipeline_setup_inputs(vsp1, pipe); - vsp1_du_pipeline_configure(pipe); + + /* + * We may get called before the pipeline gets enabled, postpone + * configuration in that case. vsp1_du_pipeline_configure() will be + * called from vsp1_du_atomic_enable(). + */ + if (drm_pipe->enabled) + vsp1_du_pipeline_configure(pipe);
done: mutex_unlock(&vsp1->drm->lock); diff --git a/drivers/media/platform/vsp1/vsp1_drm.h b/drivers/media/platform/vsp1/vsp1_drm.h index e85ad4366fbb..d780dafc1324 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.h +++ b/drivers/media/platform/vsp1/vsp1_drm.h @@ -20,6 +20,7 @@ /** * vsp1_drm_pipeline - State for the API exposed to the DRM driver * @pipe: the VSP1 pipeline used for display + * @enabled: true if the pipeline is enabled * @width: output display width * @height: output display height * @force_brx_release: when set, release the BRx during the next reconfiguration @@ -31,6 +32,7 @@ */ struct vsp1_drm_pipeline { struct vsp1_pipeline pipe; + bool enabled;
unsigned int width; unsigned int height;
The configuration API between the VSP and the DU has been updated to provide finer grain control over modesetting, and enablement.
Split rcar_du_vsp_enable() into rcar_du_vsp_modeset() and rcar_du_vsp_enable() accordingly, and update each function to use the new VSP API.
There are no further users of the deprecated vsp1_du_setup_lif() which can now be removed.
Reviewed-by: Ulrich Hecht uli+renesas@fpond.eu Signed-off-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- Changes since v2:
- Minor formatting changes
Changes since v3: - Minor formatting for checkpatch.pl --strict
- Fix inlinc typo in disabled option code paths. Reported-by: kernel test robot lkp@intel.com --- drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 4 +++- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 20 ++++++++++++++------ drivers/gpu/drm/rcar-du/rcar_du_vsp.h | 3 +++ 3 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index ea7e39d03545..53838dde2f29 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -533,8 +533,10 @@ static void rcar_du_crtc_setup(struct rcar_du_crtc *rcrtc) rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
/* Enable the VSP compositor. */ - if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) + if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) { + rcar_du_vsp_modeset(rcrtc); rcar_du_vsp_enable(rcrtc); + }
/* Turn vertical blanking interrupt reporting on. */ drm_crtc_vblank_on(&rcrtc->crtc); diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 23e41c83c875..16bd5bbf7b6c 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -47,16 +47,14 @@ static void rcar_du_vsp_complete(void *private, unsigned int status, u32 crc) drm_crtc_add_crc_entry(&crtc->crtc, false, 0, &crc); }
-void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) +void rcar_du_vsp_modeset(struct rcar_du_crtc *crtc) { const struct drm_display_mode *mode = &crtc->crtc.state->adjusted_mode; struct rcar_du_device *rcdu = crtc->dev; - struct vsp1_du_lif_config cfg = { + struct vsp1_du_modeset_config cfg = { .width = mode->hdisplay, .height = mode->vdisplay, .interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE, - .callback = rcar_du_vsp_complete, - .callback_data = crtc, }; struct rcar_du_plane_state state = { .state = { @@ -93,12 +91,22 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) */ crtc->group->need_restart = true;
- vsp1_du_setup_lif(crtc->vsp->vsp, crtc->vsp_pipe, &cfg); + vsp1_du_atomic_modeset(crtc->vsp->vsp, crtc->vsp_pipe, &cfg); +} + +void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) +{ + struct vsp1_du_enable_config cfg = { + .callback = rcar_du_vsp_complete, + .callback_data = crtc, + }; + + vsp1_du_atomic_enable(crtc->vsp->vsp, crtc->vsp_pipe, &cfg); }
void rcar_du_vsp_disable(struct rcar_du_crtc *crtc) { - vsp1_du_setup_lif(crtc->vsp->vsp, crtc->vsp_pipe, NULL); + vsp1_du_atomic_disable(crtc->vsp->vsp, crtc->vsp_pipe); }
void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h index 9b4724159378..82f2ffc9887f 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h @@ -58,6 +58,7 @@ to_rcar_vsp_plane_state(struct drm_plane_state *state) #ifdef CONFIG_DRM_RCAR_VSP int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np, unsigned int crtcs); +void rcar_du_vsp_modeset(struct rcar_du_crtc *crtc); void rcar_du_vsp_enable(struct rcar_du_crtc *crtc); void rcar_du_vsp_disable(struct rcar_du_crtc *crtc); void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc); @@ -73,6 +74,8 @@ static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp, { return -ENXIO; } + +static inline void rcar_du_vsp_modeset(struct rcar_du_crtc *crtc) { }; static inline void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) { }; static inline void rcar_du_vsp_disable(struct rcar_du_crtc *crtc) { }; static inline void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc) { };
The vsp1_du_setup_lif() function is deprecated, and the users have been removed. Remove the implementation and the associated configuration structure.
Reviewed-by: Ulrich Hecht uli+renesas@fpond.eu Signed-off-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- drivers/media/platform/vsp1/vsp1_drm.c | 46 -------------------------- include/media/vsp1.h | 22 ------------ 2 files changed, 68 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index 4132027ead6c..a48986a24c31 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -814,52 +814,6 @@ int vsp1_du_atomic_disable(struct device *dev, unsigned int pipe_index) } EXPORT_SYMBOL_GPL(vsp1_du_atomic_disable);
-/** - * vsp1_du_setup_lif - Setup the output part of the VSP pipeline - * @dev: the VSP device - * @pipe_index: the DRM pipeline index - * @cfg: the LIF configuration - * - * Configure the output part of VSP DRM pipeline for the given frame @cfg.width - * and @cfg.height. This sets up formats on the BRx source pad, the WPF sink and - * source pads, and the LIF sink pad. - * - * The @pipe_index argument selects which DRM pipeline to setup. The number of - * available pipelines depend on the VSP instance. - * - * As the media bus code on the blend unit source pad is conditioned by the - * configuration of its sink 0 pad, we also set up the formats on all blend unit - * sinks, even if the configuration will be overwritten later by - * vsp1_du_setup_rpf(). This ensures that the blend unit configuration is set to - * a well defined state. - * - * Return 0 on success or a negative error code on failure. - */ -int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, - const struct vsp1_du_lif_config *cfg) -{ - struct vsp1_du_modeset_config modes; - struct vsp1_du_enable_config enable; - int ret; - - if (!cfg) - return vsp1_du_atomic_disable(dev, pipe_index); - - modes.width = cfg->width; - modes.height = cfg->height; - modes.interlaced = cfg->interlaced; - - ret = vsp1_du_atomic_modeset(dev, pipe_index, &modes); - if (ret) - return ret; - - enable.callback = cfg->callback; - enable.callback_data = cfg->callback_data; - - return vsp1_du_atomic_enable(dev, pipe_index, &enable); -} -EXPORT_SYMBOL_GPL(vsp1_du_setup_lif); - /** * vsp1_du_atomic_begin - Prepare for an atomic update * @dev: the VSP device diff --git a/include/media/vsp1.h b/include/media/vsp1.h index 253db8029752..a4eda8c9d048 100644 --- a/include/media/vsp1.h +++ b/include/media/vsp1.h @@ -20,28 +20,6 @@ int vsp1_du_init(struct device *dev); #define VSP1_DU_STATUS_COMPLETE BIT(0) #define VSP1_DU_STATUS_WRITEBACK BIT(1)
-/** - * struct vsp1_du_lif_config - VSP LIF configuration - Deprecated - * @width: output frame width - * @height: output frame height - * @interlaced: true for interlaced pipelines - * @callback: frame completion callback function (optional). When a callback - * is provided, the VSP driver guarantees that it will be called once - * and only once for each vsp1_du_atomic_flush() call. - * @callback_data: data to be passed to the frame completion callback - */ -struct vsp1_du_lif_config { - unsigned int width; - unsigned int height; - bool interlaced; - - void (*callback)(void *data, unsigned int status, u32 crc); - void *callback_data; -}; - -int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, - const struct vsp1_du_lif_config *cfg); - /** * struct vsp1_du_modeset_config - VSP display mode configuration * @width: output frame width
Manage the power state, and initial configuration of the CRTC from the commit tail handler. CRTCs which need to be activated are taken out of standby, and any deactivated CRTCs are put into standby.
This aims at removing CRTC state tracking from the rcar_du_crtc structure. The initial configuration of the CRTC background colours and disabling of all planes is taken out of rcar_du_crtc_setup() and moved inline into rcar_du_crtc_enable(). rcar_du_crtc_get() and rcar_du_crtc_put() are kept as they are needed to configure the VSP at the correct time, this will be addressed in a separate change.
Reviewed-by: Ulrich Hecht uli+renesas@fpond.eu Signed-off-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- Changes since v2:
- Add more documentation - Keep rcar_du_crtc_get() and rcar_du_crtc_put() - Renamed rcar_du_crtc_enable() to rcar_du_crtc_exit_standby() and rcar_du_crtc_disable() to rcar_du_crtc_enter_standby() - Reword commit message
Changes since v1:
- Registers sequence confirmed unchanged - Re-ordered in the series to handle before groups - Do not merge rcar_du_crtc_setup() (now handled by _crtc_pre_commit) --- drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 90 ++++++++++++++++++++------ drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 5 ++ drivers/gpu/drm/rcar-du/rcar_du_kms.c | 4 ++ 3 files changed, 81 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 53838dde2f29..55c0e0259153 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -521,17 +521,10 @@ static void rcar_du_cmm_setup(struct drm_crtc *crtc)
static void rcar_du_crtc_setup(struct rcar_du_crtc *rcrtc) { - /* Set display off and background to black */ - rcar_du_crtc_write(rcrtc, DOOR, DOOR_RGB(0, 0, 0)); - rcar_du_crtc_write(rcrtc, BPOR, BPOR_RGB(0, 0, 0)); - /* Configure display timings and output routing */ rcar_du_crtc_set_display_timing(rcrtc); rcar_du_group_set_routing(rcrtc->group);
- /* Start with all planes disabled. */ - rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0); - /* Enable the VSP compositor. */ if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) { rcar_du_vsp_modeset(rcrtc); @@ -542,17 +535,10 @@ static void rcar_du_crtc_setup(struct rcar_du_crtc *rcrtc) drm_crtc_vblank_on(&rcrtc->crtc); }
-static int rcar_du_crtc_get(struct rcar_du_crtc *rcrtc) +static int rcar_du_crtc_exit_standby(struct rcar_du_crtc *rcrtc) { int ret;
- /* - * Guard against double-get, as the function is called from both the - * .atomic_enable() and .atomic_begin() handlers. - */ - if (rcrtc->initialized) - return 0; - ret = clk_prepare_enable(rcrtc->clock); if (ret < 0) return ret; @@ -565,8 +551,12 @@ static int rcar_du_crtc_get(struct rcar_du_crtc *rcrtc) if (ret < 0) goto error_group;
- rcar_du_crtc_setup(rcrtc); - rcrtc->initialized = true; + /* Set display off and background to black. */ + rcar_du_crtc_write(rcrtc, DOOR, DOOR_RGB(0, 0, 0)); + rcar_du_crtc_write(rcrtc, BPOR, BPOR_RGB(0, 0, 0)); + + /* Start with all planes disabled. */ + rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
return 0;
@@ -577,13 +567,29 @@ static int rcar_du_crtc_get(struct rcar_du_crtc *rcrtc) return ret; }
-static void rcar_du_crtc_put(struct rcar_du_crtc *rcrtc) +static void rcar_du_crtc_enter_standby(struct rcar_du_crtc *rcrtc) { rcar_du_group_put(rcrtc->group);
clk_disable_unprepare(rcrtc->extclock); clk_disable_unprepare(rcrtc->clock); +} + +static void rcar_du_crtc_get(struct rcar_du_crtc *rcrtc) +{ + /* + * Guard against double-get, as the function is called from both the + * .atomic_enable() and .atomic_begin() handlers. + */ + if (rcrtc->initialized) + return; + + rcar_du_crtc_setup(rcrtc); + rcrtc->initialized = true; +}
+static void rcar_du_crtc_put(struct rcar_du_crtc *rcrtc) +{ rcrtc->initialized = false; }
@@ -714,6 +720,54 @@ static int rcar_du_crtc_atomic_check(struct drm_crtc *crtc, return 0; }
+/* + * Take all CRTCs that are made active in this commit out of standby. + * CRTCs that are deactivated by the commit are untouched and will be + * put in standby by rcar_du_crtc_atomic_enter_standby(). + */ +int rcar_du_crtc_atomic_exit_standby(struct drm_device *dev, + struct drm_atomic_state *state) +{ + struct drm_crtc_state *crtc_state; + struct drm_crtc *crtc; + unsigned int i; + int ret; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc); + + if (crtc_state->active_changed && crtc_state->active) { + ret = rcar_du_crtc_exit_standby(rcrtc); + if (ret) + return ret; + } + } + + return 0; +} + +/* + * Put all CRTCs that have been deactivated by this commit in standby. + * This shall be called at the end of the commit tail handler as the + * last operation that touches the CRTC hardware. + */ +int rcar_du_crtc_atomic_enter_standby(struct drm_device *dev, + struct drm_atomic_state *state) +{ + struct drm_crtc_state *crtc_state; + struct drm_crtc *crtc; + unsigned int i; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc); + + if (crtc_state->active_changed && !crtc_state->active) + rcar_du_crtc_enter_standby(rcrtc); + } + + return 0; +} + static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *state) { diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h index 5f2940c42225..7ff0618b9998 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h @@ -109,6 +109,11 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int swindex,
void rcar_du_crtc_finish_page_flip(struct rcar_du_crtc *rcrtc);
+int rcar_du_crtc_atomic_exit_standby(struct drm_device *dev, + struct drm_atomic_state *state); +int rcar_du_crtc_atomic_enter_standby(struct drm_device *dev, + struct drm_atomic_state *state); + void rcar_du_crtc_dsysr_clr_set(struct rcar_du_crtc *rcrtc, u32 clr, u32 set);
#endif /* __RCAR_DU_CRTC_H__ */ diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index fdb8a0d127ad..ff50316b87b5 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -462,11 +462,15 @@ static void rcar_du_atomic_commit_tail(struct drm_atomic_state *old_state) }
/* Apply the atomic update. */ + rcar_du_crtc_atomic_exit_standby(dev, old_state); + drm_atomic_helper_commit_modeset_disables(dev, old_state); drm_atomic_helper_commit_planes(dev, old_state, DRM_PLANE_COMMIT_ACTIVE_ONLY); drm_atomic_helper_commit_modeset_enables(dev, old_state);
+ rcar_du_crtc_atomic_enter_standby(dev, old_state); + drm_atomic_helper_commit_hw_done(old_state); drm_atomic_helper_wait_for_flip_done(dev, old_state);
The CRTC mode setting and routing configuration are performed at the earliest of atomic enable and atomic begin, to ensure that a valid configuration is applied to the hardware before the CRTC gets enabled and before planes are setup (the latter being required in particular by the VSP). This requires the rcar_du_crtc structure to track the CRTC enabled state.
Simplify the code and remove the manual state tracking by moving CRTC setup to a new rcar_du_crtc_atomic_modeset() function, called directly from the commit tail handler. The function iterates over all CRTCs in the state transaction and performs CRTC mode setting, routing configuration and VSP configuration.
drm_crtc_vblank_on() has to be called from the atomic begin handler, to ensure that vertical blanking reporting is enabled before the call to drm_crtc_vblank_get() in the atomic flush handler. As the drm_crtc_vblank_on() and drm_crtc_vblank_off() calls don't need to be balanced this is not an issue. A balanced alternative would have been to call drm_crtc_vblank_on() and drm_crtc_vblank_off() from the CRTC exit and enter standby handlers respectively, but drm_atomic_helper_commit_modeset_disables() complains if drm_crtc_vblank_off() hasn't been called by the atomic disable handler.
As a result, the vsp1_du_atomic_flush() operation can be called with the DRM pipeline disabled. Correct operation has been ensured by "media: vsp1: drm: Don't configure hardware when the pipeline is disabled".
Reviewed-by: Ulrich Hecht uli+renesas@fpond.eu Signed-off-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- Changes since v2:
- Deconstruct rcar_du_crtc_setup() completely --- drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 89 +++++++++++--------------- drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 4 +- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 1 + 3 files changed, 42 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 55c0e0259153..7ca721e6b9d7 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -519,22 +519,6 @@ static void rcar_du_cmm_setup(struct drm_crtc *crtc) * Start/Stop and Suspend/Resume */
-static void rcar_du_crtc_setup(struct rcar_du_crtc *rcrtc) -{ - /* Configure display timings and output routing */ - rcar_du_crtc_set_display_timing(rcrtc); - rcar_du_group_set_routing(rcrtc->group); - - /* Enable the VSP compositor. */ - if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) { - rcar_du_vsp_modeset(rcrtc); - rcar_du_vsp_enable(rcrtc); - } - - /* Turn vertical blanking interrupt reporting on. */ - drm_crtc_vblank_on(&rcrtc->crtc); -} - static int rcar_du_crtc_exit_standby(struct rcar_du_crtc *rcrtc) { int ret; @@ -575,24 +559,6 @@ static void rcar_du_crtc_enter_standby(struct rcar_du_crtc *rcrtc) clk_disable_unprepare(rcrtc->clock); }
-static void rcar_du_crtc_get(struct rcar_du_crtc *rcrtc) -{ - /* - * Guard against double-get, as the function is called from both the - * .atomic_enable() and .atomic_begin() handlers. - */ - if (rcrtc->initialized) - return; - - rcar_du_crtc_setup(rcrtc); - rcrtc->initialized = true; -} - -static void rcar_du_crtc_put(struct rcar_du_crtc *rcrtc) -{ - rcrtc->initialized = false; -} - static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc) { bool interlaced; @@ -768,6 +734,40 @@ int rcar_du_crtc_atomic_enter_standby(struct drm_device *dev, return 0; }
+/* + * Configure the mode for all CRTCs that require it. For now we only handle mode + * set on the VSP, CRTC configuration will be handled later. + */ +int rcar_du_crtc_atomic_modeset(struct drm_device *dev, + struct drm_atomic_state *state) +{ + struct drm_crtc_state *crtc_state; + struct drm_crtc *crtc; + unsigned int i; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc); + + /* + * Skip commits that don't change the mode. We manually skip + * inactive CRTCs as disabling a CRTC is considered as a mode + * set by drm_atomic_crtc_needs_modeset(). + */ + if (!drm_atomic_crtc_needs_modeset(crtc_state) || + !crtc_state->active) + continue; + + /* Configure display timings and output routing. */ + rcar_du_crtc_set_display_timing(rcrtc); + rcar_du_group_set_routing(rcrtc->group); + + if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) + rcar_du_vsp_modeset(rcrtc); + } + + return 0; +} + static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *state) { @@ -777,8 +777,6 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
if (rcrtc->cmm) rcar_cmm_enable(rcrtc->cmm); - rcar_du_crtc_get(rcrtc); - /* * On D3/E3 the dot clock is provided by the LVDS encoder attached to * the DU channel. We need to enable its clock output explicitly if @@ -793,6 +791,9 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc, rcar_lvds_clk_enable(bridge, mode->clock * 1000); }
+ if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) + rcar_du_vsp_enable(rcrtc); + rcar_du_crtc_start(rcrtc);
/* @@ -813,7 +814,6 @@ static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc, struct rcar_du_device *rcdu = rcrtc->dev;
rcar_du_crtc_stop(rcrtc); - rcar_du_crtc_put(rcrtc);
if (rcdu->info->lvds_clk_mask & BIT(rcrtc->index) && rstate->outputs == BIT(RCAR_DU_OUTPUT_DPAD0)) { @@ -841,19 +841,8 @@ static void rcar_du_crtc_atomic_begin(struct drm_crtc *crtc,
WARN_ON(!crtc->state->enable);
- /* - * If a mode set is in progress we can be called with the CRTC disabled. - * We thus need to first get and setup the CRTC in order to configure - * planes. We must *not* put the CRTC in .atomic_flush(), as it must be - * kept awake until the .atomic_enable() call that will follow. The get - * operation in .atomic_enable() will in that case be a no-op, and the - * CRTC will be put later in .atomic_disable(). - * - * If a mode set is not in progress the CRTC is enabled, and the - * following get call will be a no-op. There is thus no need to balance - * it in .atomic_flush() either. - */ - rcar_du_crtc_get(rcrtc); + /* Turn vertical blanking interrupt reporting on. */ + drm_crtc_vblank_on(&rcrtc->crtc);
/* If the active state changed, we let .atomic_enable handle CMM. */ if (crtc->state->color_mgmt_changed && !crtc->state->active_changed) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h index 7ff0618b9998..2b58316cca2b 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h @@ -30,7 +30,6 @@ struct rcar_du_vsp; * @extclock: external pixel dot clock (optional) * @mmio_offset: offset of the CRTC registers in the DU MMIO block * @index: CRTC hardware index - * @initialized: whether the CRTC has been initialized and clocks enabled * @dsysr: cached value of the DSYSR register * @vblank_enable: whether vblank events are enabled on this CRTC * @event: event to post when the pending page flip completes @@ -52,7 +51,6 @@ struct rcar_du_crtc { struct clk *extclock; unsigned int mmio_offset; unsigned int index; - bool initialized;
u32 dsysr;
@@ -113,6 +111,8 @@ int rcar_du_crtc_atomic_exit_standby(struct drm_device *dev, struct drm_atomic_state *state); int rcar_du_crtc_atomic_enter_standby(struct drm_device *dev, struct drm_atomic_state *state); +int rcar_du_crtc_atomic_modeset(struct drm_device *dev, + struct drm_atomic_state *state);
void rcar_du_crtc_dsysr_clr_set(struct rcar_du_crtc *rcrtc, u32 clr, u32 set);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index ff50316b87b5..3c10c329c81c 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -465,6 +465,7 @@ static void rcar_du_atomic_commit_tail(struct drm_atomic_state *old_state) rcar_du_crtc_atomic_exit_standby(dev, old_state);
drm_atomic_helper_commit_modeset_disables(dev, old_state); + rcar_du_crtc_atomic_modeset(dev, old_state); drm_atomic_helper_commit_planes(dev, old_state, DRM_PLANE_COMMIT_ACTIVE_ONLY); drm_atomic_helper_commit_modeset_enables(dev, old_state);
Refactoring of the group control code will soon require more iteration over the available groups. Simplify this process by introducing a group iteration helper.
Reviewed-by: Ulrich Hecht uli+renesas@fpond.eu Signed-off-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- Changes since v2:
- Don't assign __group in the condition part of the for statement of the for_each_rcdu_group() macro --- drivers/gpu/drm/rcar-du/rcar_du_drv.h | 5 +++++ drivers/gpu/drm/rcar-du/rcar_du_kms.c | 10 ++-------- 2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index 02ca2d0e1b55..e792ba7f5145 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h @@ -104,6 +104,11 @@ static inline struct rcar_du_device *to_rcar_du_device(struct drm_device *dev) return container_of(dev, struct rcar_du_device, ddev); }
+#define for_each_rcdu_group(__rcdu, __group, __i) \ + for ((__i) = 0, (__group) = &(__rcdu)->groups[0]; \ + (__i) < DIV_ROUND_UP((__rcdu)->num_crtcs, 2); \ + __i++, __group++) + static inline bool rcar_du_has(struct rcar_du_device *rcdu, unsigned int feature) { diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 3c10c329c81c..732aac342dab 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -771,9 +771,9 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
struct drm_device *dev = &rcdu->ddev; struct drm_encoder *encoder; + struct rcar_du_group *rgrp; unsigned int dpad0_sources; unsigned int num_encoders; - unsigned int num_groups; unsigned int swindex; unsigned int hwindex; unsigned int i; @@ -820,11 +820,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) return ret;
/* Initialize the groups. */ - num_groups = DIV_ROUND_UP(rcdu->num_crtcs, 2); - - for (i = 0; i < num_groups; ++i) { - struct rcar_du_group *rgrp = &rcdu->groups[i]; - + for_each_rcdu_group(rcdu, rgrp, i) { mutex_init(&rgrp->lock);
rgrp->dev = rcdu; @@ -866,8 +862,6 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
/* Create the CRTCs. */ for (swindex = 0, hwindex = 0; swindex < rcdu->num_crtcs; ++hwindex) { - struct rcar_du_group *rgrp; - /* Skip unpopulated DU channels. */ if (!(rcdu->info->channels_mask & BIT(hwindex))) continue;
Create a new private state object for the DU groups, and move the initialisation of a group object to a new function rcar_du_group_init().
Reviewed-by: Ulrich Hecht uli+renesas@fpond.eu Signed-off-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- Changes since v3: - Rebase to v5.11 - Fix pointer passing for drm_atomic_private_obj_init() - Fix checkpatch.pl --strict warning about if (state == NULL)
Changes since v2:
- Call mutex_destroy() when cleaning up the group - Include mutex.h and slab.h - Squash "drm: rcar-du: Add rcar_du_get_{old,new}_group_state()" --- drivers/gpu/drm/rcar-du/rcar_du_group.c | 144 ++++++++++++++++++++++++ drivers/gpu/drm/rcar-du/rcar_du_group.h | 29 +++++ drivers/gpu/drm/rcar-du/rcar_du_kms.c | 27 +---- 3 files changed, 177 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c index 88a783ceb3e9..7c13def703b7 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c @@ -25,6 +25,11 @@
#include <linux/clk.h> #include <linux/io.h> +#include <linux/slab.h> + +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_state_helper.h> +#include <drm/drm_device.h>
#include "rcar_du_drv.h" #include "rcar_du_group.h" @@ -361,3 +366,142 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
return rcar_du_set_dpad0_vsp1_routing(rgrp->dev); } + +/* ----------------------------------------------------------------------------- + * Group State Handling + */ + +static struct drm_private_state * +rcar_du_group_atomic_duplicate_state(struct drm_private_obj *obj) +{ + struct rcar_du_group_state *state; + + if (WARN_ON(!obj->state)) + return NULL; + + state = kzalloc(sizeof(*state), GFP_KERNEL); + if (!state) + return NULL; + + __drm_atomic_helper_private_obj_duplicate_state(obj, &state->state); + + return &state->state; +} + +static void rcar_du_group_atomic_destroy_state(struct drm_private_obj *obj, + struct drm_private_state *state) +{ + kfree(to_rcar_group_state(state)); +} + +static const struct drm_private_state_funcs rcar_du_group_state_funcs = { + .atomic_duplicate_state = rcar_du_group_atomic_duplicate_state, + .atomic_destroy_state = rcar_du_group_atomic_destroy_state, +}; + +/** + * rcar_du_get_old_group_state - get old group state, if it exists + * @state: global atomic state object + * @rgrp: group to grab + * + * This function returns the old group state for the given group, or + * NULL if the group is not part of the global atomic state. + */ +struct rcar_du_group_state * +rcar_du_get_old_group_state(struct drm_atomic_state *state, + struct rcar_du_group *rgrp) +{ + struct drm_private_obj *obj = &rgrp->private; + struct drm_private_state *pstate; + unsigned int i; + + for (i = 0; i < state->num_private_objs; i++) { + if (obj == state->private_objs[i].ptr) { + pstate = state->private_objs[i].old_state; + return to_rcar_group_state(pstate); + } + } + + return NULL; +} + +/** + * rcar_du_get_new_group_state - get new group state, if it exists + * @state: global atomic state object + * @rgrp: group to grab + * + * This function returns the new group state for the given group, or + * NULL if the group is not part of the global atomic state. + */ +struct rcar_du_group_state * +rcar_du_get_new_group_state(struct drm_atomic_state *state, + struct rcar_du_group *rgrp) +{ + struct drm_private_obj *obj = &rgrp->private; + struct drm_private_state *pstate; + unsigned int i; + + for (i = 0; i < state->num_private_objs; i++) { + if (obj == state->private_objs[i].ptr) { + pstate = state->private_objs[i].new_state; + return to_rcar_group_state(pstate); + } + } + + return NULL; +} + +/* ----------------------------------------------------------------------------- + * Init and Cleanup + */ + +/* + * rcar_du_group_init - Initialise and reset a group object + * + * Return 0 in case of success or a negative error code otherwise. + */ +int rcar_du_group_init(struct rcar_du_device *rcdu, struct rcar_du_group *rgrp, + unsigned int index) +{ + static const unsigned int mmio_offsets[] = { + DU0_REG_OFFSET, DU2_REG_OFFSET + }; + + struct rcar_du_group_state *state; + + state = kzalloc(sizeof(*state), GFP_KERNEL); + if (!state) + return -ENOMEM; + + drm_atomic_private_obj_init(&rcdu->ddev, &rgrp->private, &state->state, + &rcar_du_group_state_funcs); + + mutex_init(&rgrp->lock); + + rgrp->dev = rcdu; + rgrp->mmio_offset = mmio_offsets[index]; + rgrp->index = index; + /* Extract the channel mask for this group only. */ + rgrp->channels_mask = (rcdu->info->channels_mask >> (2 * index)) + & GENMASK(1, 0); + rgrp->num_crtcs = hweight8(rgrp->channels_mask); + + /* + * If we have more than one CRTC in this group pre-associate + * the low-order planes with CRTC 0 and the high-order planes + * with CRTC 1 to minimize flicker occurring when the + * association is changed. + */ + rgrp->dptsr_planes = rgrp->num_crtcs > 1 + ? (rcdu->info->gen >= 3 ? 0x04 : 0xf0) + : 0; + + return 0; +} + +void rcar_du_group_cleanup(struct rcar_du_group *rgrp) +{ + mutex_destroy(&rgrp->lock); + + drm_atomic_private_obj_fini(&rgrp->private); +} diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.h b/drivers/gpu/drm/rcar-du/rcar_du_group.h index e9906609c635..f0b94eb77746 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_group.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.h @@ -12,12 +12,15 @@
#include <linux/mutex.h>
+#include <drm/drm_atomic.h> + #include "rcar_du_plane.h"
struct rcar_du_device;
/* * struct rcar_du_group - CRTCs and planes group + * @private: The base drm private object * @dev: the DU device * @mmio_offset: registers offset in the device memory map * @index: group index @@ -33,6 +36,8 @@ struct rcar_du_device; * @need_restart: the group needs to be restarted due to a configuration change */ struct rcar_du_group { + struct drm_private_obj private; + struct rcar_du_device *dev; unsigned int mmio_offset; unsigned int index; @@ -51,6 +56,19 @@ struct rcar_du_group { bool need_restart; };
+#define to_rcar_group(s) container_of(s, struct rcar_du_group, private) + +/** + * struct rcar_du_group_state - Driver-specific group state + * @state: base DRM private state + */ +struct rcar_du_group_state { + struct drm_private_state state; +}; + +#define to_rcar_group_state(s) \ + container_of(s, struct rcar_du_group_state, state) + u32 rcar_du_group_read(struct rcar_du_group *rgrp, u32 reg); void rcar_du_group_write(struct rcar_du_group *rgrp, u32 reg, u32 data);
@@ -62,4 +80,15 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp);
int rcar_du_set_dpad0_vsp1_routing(struct rcar_du_device *rcdu);
+struct rcar_du_group_state * +rcar_du_get_old_group_state(struct drm_atomic_state *state, + struct rcar_du_group *rgrp); +struct rcar_du_group_state * +rcar_du_get_new_group_state(struct drm_atomic_state *state, + struct rcar_du_group *rgrp); + +int rcar_du_group_init(struct rcar_du_device *rcdu, struct rcar_du_group *rgrp, + unsigned int index); +void rcar_du_group_cleanup(struct rcar_du_group *rgrp); + #endif /* __RCAR_DU_GROUP_H__ */ diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 732aac342dab..d3069fb284fb 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -26,6 +26,7 @@ #include "rcar_du_crtc.h" #include "rcar_du_drv.h" #include "rcar_du_encoder.h" +#include "rcar_du_group.h" #include "rcar_du_kms.h" #include "rcar_du_regs.h" #include "rcar_du_vsp.h" @@ -765,10 +766,6 @@ static void rcar_du_modeset_cleanup(struct drm_device *dev, void *res)
int rcar_du_modeset_init(struct rcar_du_device *rcdu) { - static const unsigned int mmio_offsets[] = { - DU0_REG_OFFSET, DU2_REG_OFFSET - }; - struct drm_device *dev = &rcdu->ddev; struct drm_encoder *encoder; struct rcar_du_group *rgrp; @@ -821,25 +818,9 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
/* Initialize the groups. */ for_each_rcdu_group(rcdu, rgrp, i) { - mutex_init(&rgrp->lock); - - rgrp->dev = rcdu; - rgrp->mmio_offset = mmio_offsets[i]; - rgrp->index = i; - /* Extract the channel mask for this group only. */ - rgrp->channels_mask = (rcdu->info->channels_mask >> (2 * i)) - & GENMASK(1, 0); - rgrp->num_crtcs = hweight8(rgrp->channels_mask); - - /* - * If we have more than one CRTCs in this group pre-associate - * the low-order planes with CRTC 0 and the high-order planes - * with CRTC 1 to minimize flicker occurring when the - * association is changed. - */ - rgrp->dptsr_planes = rgrp->num_crtcs > 1 - ? (rcdu->info->gen >= 3 ? 0x04 : 0xf0) - : 0; + ret = rcar_du_group_init(rcdu, rgrp, i); + if (ret < 0) + return ret;
if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE)) { ret = rcar_du_planes_init(rgrp);
Create rcar_du_group_atomic_check() and rcar_du_group_atomic_setup() functions to track and apply group state through the DRM atomic state. The use_count field is moved from the rcar_du_group structure to an enabled field in the rcar_du_group_state structure.
This allows separating group setup from the configuration of the CRTCs that are part of the group, simplifying the CRTC code and improving overall readability. The existing rcar_du_group_{get,put}() functions are now redundant and removed.
The groups share clocking with the CRTCs within the group, and so are accessible only when one of its CRTCs has been powered through rcar_du_crtc_atomic_exit_standby().
Reviewed-by: Ulrich Hecht uli+renesas@fpond.eu Signed-off-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- Changes since v3: - Shorted macro length, and added relevant documentation to its usage and function.
Changes since v2:
- Simplify error handling in rcar_du_crtc_enable() - Rename rcar_du_group_atomic_pre_commit() to rcar_du_group_atomic_setup() and turn it into a void function - Remove rcar_du_group_atomic_post_commit() - Replace group state use_count field by enabled - Rename group state variable from rstate to gstate
Changes since v1:
- All register sequences now maintained. - Clock management is no longer handled by the group (_crtc_{exit,enter}_standby handles this for us) --- drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 18 +---- drivers/gpu/drm/rcar-du/rcar_du_group.c | 102 ++++++++++++++++-------- drivers/gpu/drm/rcar-du/rcar_du_group.h | 12 ++- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 5 ++ 4 files changed, 87 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 7ca721e6b9d7..020eb75732a7 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -528,12 +528,10 @@ static int rcar_du_crtc_exit_standby(struct rcar_du_crtc *rcrtc) return ret;
ret = clk_prepare_enable(rcrtc->extclock); - if (ret < 0) - goto error_clock; - - ret = rcar_du_group_get(rcrtc->group); - if (ret < 0) - goto error_group; + if (ret < 0) { + clk_disable_unprepare(rcrtc->clock); + return ret; + }
/* Set display off and background to black. */ rcar_du_crtc_write(rcrtc, DOOR, DOOR_RGB(0, 0, 0)); @@ -543,18 +541,10 @@ static int rcar_du_crtc_exit_standby(struct rcar_du_crtc *rcrtc) rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
return 0; - -error_group: - clk_disable_unprepare(rcrtc->extclock); -error_clock: - clk_disable_unprepare(rcrtc->clock); - return ret; }
static void rcar_du_crtc_enter_standby(struct rcar_du_crtc *rcrtc) { - rcar_du_group_put(rcrtc->group); - clk_disable_unprepare(rcrtc->extclock); clk_disable_unprepare(rcrtc->clock); } diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c index 7c13def703b7..fdd4a60f5f5e 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c @@ -24,6 +24,7 @@ */
#include <linux/clk.h> +#include <linux/err.h> #include <linux/io.h> #include <linux/slab.h>
@@ -183,38 +184,6 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp) mutex_unlock(&rgrp->lock); }
-/* - * rcar_du_group_get - Acquire a reference to the DU channels group - * - * Acquiring the first reference setups core registers. A reference must be held - * before accessing any hardware registers. - * - * This function must be called with the DRM mode_config lock held. - * - * Return 0 in case of success or a negative error code otherwise. - */ -int rcar_du_group_get(struct rcar_du_group *rgrp) -{ - if (rgrp->use_count) - goto done; - - rcar_du_group_setup(rgrp); - -done: - rgrp->use_count++; - return 0; -} - -/* - * rcar_du_group_put - Release a reference to the DU - * - * This function must be called with the DRM mode_config lock held. - */ -void rcar_du_group_put(struct rcar_du_group *rgrp) -{ - --rgrp->use_count; -} - static void __rcar_du_group_start_stop(struct rcar_du_group *rgrp, bool start) { struct rcar_du_device *rcdu = rgrp->dev; @@ -399,6 +368,34 @@ static const struct drm_private_state_funcs rcar_du_group_state_funcs = { .atomic_destroy_state = rcar_du_group_atomic_destroy_state, };
+/** + * for_each_oldnew_group_in_state - iterate over all groups in an atomic update + * @__state: &struct drm_atomic_state pointer + * @__obj: &struct drm_private_obj iteration cursor + * @__old: &struct drm_private_state iteration cursor for the old state + * @__new: &struct drm_private_state iteration cursor for the new state + * @__i: unsigned int iteration cursor, for macro-internal use + * + * Iterate over all private objects in an atomic update, processing only those + * which represent rcar_du_group_state, tracking both old and new state. + */ +#define for_each_oldnew_group_in_state(__state, __obj, __old, __new, __i) \ + for_each_oldnew_private_obj_in_state((__state), (__obj), (__old), (__new), (__i)) \ + for_each_if((__obj)->funcs == &rcar_du_group_state_funcs) + +static struct rcar_du_group_state * +rcar_du_get_group_state(struct drm_atomic_state *state, + struct rcar_du_group *rgrp) +{ + struct drm_private_state *pstate; + + pstate = drm_atomic_get_private_obj_state(state, &rgrp->private); + if (IS_ERR(pstate)) + return ERR_CAST(pstate); + + return to_rcar_group_state(pstate); +} + /** * rcar_du_get_old_group_state - get old group state, if it exists * @state: global atomic state object @@ -451,6 +448,47 @@ rcar_du_get_new_group_state(struct drm_atomic_state *state, return NULL; }
+int rcar_du_group_atomic_check(struct drm_device *dev, + struct drm_atomic_state *state) +{ + struct drm_crtc_state *crtc_state; + struct drm_crtc *crtc; + unsigned int i; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc); + struct rcar_du_group_state *gstate; + + gstate = rcar_du_get_group_state(state, rcrtc->group); + if (IS_ERR(gstate)) + return PTR_ERR(gstate); + + if (crtc_state->active) + gstate->enabled = true; + } + + return 0; +} + +void rcar_du_group_atomic_setup(struct drm_device *dev, + struct drm_atomic_state *state) +{ + struct drm_private_state *old_pstate, *new_pstate; + struct drm_private_obj *obj; + unsigned int i; + + for_each_oldnew_group_in_state(state, obj, old_pstate, new_pstate, i) { + struct rcar_du_group *rgrp = to_rcar_group(obj); + struct rcar_du_group_state *old_state, *new_state; + + old_state = to_rcar_group_state(old_pstate); + new_state = to_rcar_group_state(new_pstate); + + if (!old_state->enabled && new_state->enabled) + rcar_du_group_setup(rgrp); + } +} + /* ----------------------------------------------------------------------------- * Init and Cleanup */ diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.h b/drivers/gpu/drm/rcar-du/rcar_du_group.h index f0b94eb77746..c6e0ae79ede9 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_group.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.h @@ -27,7 +27,6 @@ struct rcar_du_device; * @channels_mask: bitmask of populated DU channels in this group * @cmms_mask: bitmask of available CMMs in this group * @num_crtcs: number of CRTCs in this group (1 or 2) - * @use_count: number of users of the group (rcar_du_group_(get|put)) * @used_crtcs: number of CRTCs currently in use * @lock: protects the dptsr_planes field and the DPTSR register * @dptsr_planes: bitmask of planes driven by dot-clock and timing generator 1 @@ -45,7 +44,6 @@ struct rcar_du_group { unsigned int channels_mask; unsigned int cmms_mask; unsigned int num_crtcs; - unsigned int use_count; unsigned int used_crtcs;
struct mutex lock; @@ -61,9 +59,12 @@ struct rcar_du_group { /** * struct rcar_du_group_state - Driver-specific group state * @state: base DRM private state + * @enabled: true if at least one CRTC in the group is enabled */ struct rcar_du_group_state { struct drm_private_state state; + + bool enabled; };
#define to_rcar_group_state(s) \ @@ -72,8 +73,6 @@ struct rcar_du_group_state { u32 rcar_du_group_read(struct rcar_du_group *rgrp, u32 reg); void rcar_du_group_write(struct rcar_du_group *rgrp, u32 reg, u32 data);
-int rcar_du_group_get(struct rcar_du_group *rgrp); -void rcar_du_group_put(struct rcar_du_group *rgrp); void rcar_du_group_start_stop(struct rcar_du_group *rgrp, bool start); void rcar_du_group_restart(struct rcar_du_group *rgrp); int rcar_du_group_set_routing(struct rcar_du_group *rgrp); @@ -87,6 +86,11 @@ struct rcar_du_group_state * rcar_du_get_new_group_state(struct drm_atomic_state *state, struct rcar_du_group *rgrp);
+int rcar_du_group_atomic_check(struct drm_device *dev, + struct drm_atomic_state *state); +void rcar_du_group_atomic_setup(struct drm_device *dev, + struct drm_atomic_state *state); + int rcar_du_group_init(struct rcar_du_device *rcdu, struct rcar_du_group *rgrp, unsigned int index); void rcar_du_group_cleanup(struct rcar_du_group *rgrp); diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index d3069fb284fb..8a0c26939de2 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -430,6 +430,10 @@ static int rcar_du_atomic_check(struct drm_device *dev, if (ret) return ret;
+ ret = rcar_du_group_atomic_check(dev, state); + if (ret) + return ret; + if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE)) return 0;
@@ -464,6 +468,7 @@ static void rcar_du_atomic_commit_tail(struct drm_atomic_state *old_state)
/* Apply the atomic update. */ rcar_du_crtc_atomic_exit_standby(dev, old_state); + rcar_du_group_atomic_setup(dev, old_state);
drm_atomic_helper_commit_modeset_disables(dev, old_state); rcar_du_crtc_atomic_modeset(dev, old_state);
Hi Kieran,
On Mon, Mar 22, 2021 at 04:35:34PM +0000, Kieran Bingham wrote:
Create rcar_du_group_atomic_check() and rcar_du_group_atomic_setup() functions to track and apply group state through the DRM atomic state. The use_count field is moved from the rcar_du_group structure to an enabled field in the rcar_du_group_state structure.
This allows separating group setup from the configuration of the CRTCs that are part of the group, simplifying the CRTC code and improving overall readability. The existing rcar_du_group_{get,put}() functions are now redundant and removed.
The groups share clocking with the CRTCs within the group, and so are accessible only when one of its CRTCs has been powered through rcar_du_crtc_atomic_exit_standby().
Reviewed-by: Ulrich Hecht uli+renesas@fpond.eu Signed-off-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
It's a bit weird to have both your and Laurent's SoB without a Co-Developped-By or an authorship from him.
However, using a drm_private_obj shared between CRTC has a gotcha: you don't have any ordering guarantee between commits if they affect different CRTCs (and they are non-blocking).
Let's assume we have two subsequent commits, commit1 and commit2, both non-blocking, and affecting different CRTC, plane and connectors. In this case, commit1 old private state will be commit0 new private state, and commit 2 old private state will be commit1 new private state.
If commit2 is executed before commit1, then it will free its old state when done with it (so commit1 new private state), and will thus result in an use-after-free when commit1 is ran.
In order to fix this, you should store (and get a reference to) the drm_crtc_commit in your private state in atomic_commit_setup, and call drm_crtc_commit_wait on that commit as the first part of your commit_tail to serialize those commits.
Maxime
From: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
Routing configuration for the DU is complex. Depending on the SoC generation various routing options are available:
- The VSP to DU routing is not available on Gen1, is configurable on Gen2 and is fixed on Gen3. When configurable, the routing affects both CRTC groups but is set in a register of the first CRTC group. - The DU channel to DPAD output routing is explicitly configurable on some SoCs of the Gen2 and Gen3 family. When configurable, the DPAD outputs never offer routing options to CRTCs belonging to different groups. - On all SoCs the routing of DU channels to DU pin controllers (internal output of the DU channels) can be swapped within a group. This feature is only used on Gen1 to control routing of the DPAD1 output.
Routing is thus handled at the group level, but for Gen2 hardware requires configuration of the DPAD0 and VSPD1 routing in the first group even when only the second group is enabled.
Routing at the group level is currently configured when applying CRTC configuration. Global routing is configured at the same time, and is additionally configured by the plane setup code to set VSPD1 routing. This results in code paths that are difficult to follow.
Simplify the routing configuration by performing it all directly, based on CRTC and CRTC group state. Group-level routing is moved to group setup as it only depends on the group state and the state of the CRTCs it contains. Global routing is moved to the commit tail handler, and based on global DU state.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Signed-off-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com --- Changes since v3: - s/DPAD1/DPAD0/ - s/VSP1D/VSPD1/ --- drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 3 +- drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 - drivers/gpu/drm/rcar-du/rcar_du_group.c | 156 ++++++++++++++++-------- drivers/gpu/drm/rcar-du/rcar_du_group.h | 3 +- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 16 +-- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 10 +- 6 files changed, 116 insertions(+), 73 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 020eb75732a7..b5f07fd9c4be 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -747,9 +747,8 @@ int rcar_du_crtc_atomic_modeset(struct drm_device *dev, !crtc_state->active) continue;
- /* Configure display timings and output routing. */ + /* Configure display timings. */ rcar_du_crtc_set_display_timing(rcrtc); - rcar_du_group_set_routing(rcrtc->group);
if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) rcar_du_vsp_modeset(rcrtc); diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index e792ba7f5145..ed0b57722f37 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h @@ -95,7 +95,6 @@ struct rcar_du_device { } props;
unsigned int dpad0_source; - unsigned int dpad1_source; unsigned int vspd1_sink; };
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c index fdd4a60f5f5e..368676507097 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c @@ -46,6 +46,10 @@ void rcar_du_group_write(struct rcar_du_group *rgrp, u32 reg, u32 data) rcar_du_write(rgrp->dev, rgrp->mmio_offset + reg, data); }
+/* ----------------------------------------------------------------------------- + * Static Group Setup + */ + static void rcar_du_group_setup_pins(struct rcar_du_group *rgrp) { u32 defr6 = DEFR6_CODE; @@ -59,37 +63,6 @@ static void rcar_du_group_setup_pins(struct rcar_du_group *rgrp) rcar_du_group_write(rgrp, DEFR6, defr6); }
-static void rcar_du_group_setup_defr8(struct rcar_du_group *rgrp) -{ - struct rcar_du_device *rcdu = rgrp->dev; - u32 defr8 = DEFR8_CODE; - - if (rcdu->info->gen < 3) { - defr8 |= DEFR8_DEFE8; - - /* - * On Gen2 the DEFR8 register for the first group also controls - * RGB output routing to DPAD0 and VSPD1 routing to DU0/1/2 for - * DU instances that support it. - */ - if (rgrp->index == 0) { - defr8 |= DEFR8_DRGBS_DU(rcdu->dpad0_source); - if (rgrp->dev->vspd1_sink == 2) - defr8 |= DEFR8_VSCS; - } - } else { - /* - * On Gen3 VSPD routing can't be configured, and DPAD routing - * is set in the group corresponding to the DPAD output (no Gen3 - * SoC has multiple DPAD sources belonging to separate groups). - */ - if (rgrp->index == rcdu->dpad0_source / 2) - defr8 |= DEFR8_DRGBS_DU(rcdu->dpad0_source); - } - - rcar_du_group_write(rgrp, DEFR8, defr8); -} - static void rcar_du_group_setup_didsr(struct rcar_du_group *rgrp) { struct rcar_du_device *rcdu = rgrp->dev; @@ -163,10 +136,8 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp) (rgrp->cmms_mask & BIT(0) ? DEFR7_CMME0 : 0); rcar_du_group_write(rgrp, DEFR7, defr7);
- if (rcdu->info->gen >= 2) { - rcar_du_group_setup_defr8(rgrp); + if (rcdu->info->gen >= 2) rcar_du_group_setup_didsr(rgrp); - }
if (rcdu->info->gen >= 3) rcar_du_group_write(rgrp, DEFR10, DEFR10_CODE | DEFR10_DEFE10); @@ -184,6 +155,10 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp) mutex_unlock(&rgrp->lock); }
+/* ----------------------------------------------------------------------------- + * Start & Stop + */ + static void __rcar_du_group_start_stop(struct rcar_du_group *rgrp, bool start) { struct rcar_du_device *rcdu = rgrp->dev; @@ -239,26 +214,63 @@ void rcar_du_group_restart(struct rcar_du_group *rgrp) __rcar_du_group_start_stop(rgrp, true); }
+/* ----------------------------------------------------------------------------- + * Input and Output Routing + */ + +static void rcar_du_group_setup_defr8(struct rcar_du_group *rgrp) +{ + struct rcar_du_device *rcdu = rgrp->dev; + u32 defr8 = DEFR8_CODE; + + if (rcdu->info->gen < 3) { + defr8 |= DEFR8_DEFE8; + + /* + * On Gen2 the DEFR8 register for the first group also controls + * RGB output routing to DPAD0 and VSPD1 routing to DU0/1/2 for + * DU instances that support it. + */ + if (rgrp->index == 0) { + defr8 |= DEFR8_DRGBS_DU(rcdu->dpad0_source); + if (rgrp->dev->vspd1_sink == 2) + defr8 |= DEFR8_VSCS; + } + } else { + /* + * On Gen3 VSPD routing can't be configured, and DPAD routing + * is set in the group corresponding to the DPAD output (no Gen3 + * SoC has multiple DPAD sources belonging to separate groups). + */ + if (rgrp->index == rcdu->dpad0_source / 2) + defr8 |= DEFR8_DRGBS_DU(rcdu->dpad0_source); + } + + rcar_du_group_write(rgrp, DEFR8, defr8); +} + int rcar_du_set_dpad0_vsp1_routing(struct rcar_du_device *rcdu) { struct rcar_du_group *rgrp; struct rcar_du_crtc *crtc; - unsigned int index; int ret;
- if (rcdu->info->gen < 2) + /* + * Only Gen2 hardware has global routing not handled in the group that + * holds the corresponding CRTCs. + */ + if (rcdu->info->gen != 2) return 0;
/* - * RGB output routing to DPAD0 and VSP1D routing to DU0/1/2 are - * configured in the DEFR8 register of the first group on Gen2 and the - * last group on Gen3. As this function can be called with the DU - * channels of the corresponding CRTCs disabled, we need to enable the - * group clock before accessing the register. + * RGB output routing to DPAD0 and VSPD1 routing to DU0/1/2 are + * configured in the DEFR8 register of the first group on Gen2. As this + * function can be called with the DU channels of the corresponding + * CRTCs disabled, we need to enable the group clock before accessing + * the register. */ - index = rcdu->info->gen < 3 ? 0 : DIV_ROUND_UP(rcdu->num_crtcs, 2) - 1; - rgrp = &rcdu->groups[index]; - crtc = &rcdu->crtcs[index * 2]; + rgrp = &rcdu->groups[0]; + crtc = &rcdu->crtcs[0];
ret = clk_prepare_enable(crtc->clock); if (ret < 0) @@ -312,19 +324,33 @@ static void rcar_du_group_set_dpad_levels(struct rcar_du_group *rgrp) rcar_du_group_write(rgrp, DOFLR, doflr); }
-int rcar_du_group_set_routing(struct rcar_du_group *rgrp) +static void rcar_du_group_set_routing(struct rcar_du_group *rgrp) { struct rcar_du_device *rcdu = rgrp->dev; u32 dorcr = rcar_du_group_read(rgrp, DORCR); + bool sp1_to_pin2 = false;
dorcr &= ~(DORCR_PG2T | DORCR_DK2S | DORCR_PG2D_MASK);
/* - * Set the DPAD1 pins sources. Select CRTC 0 if explicitly requested and - * CRTC 1 in all other cases to avoid cloning CRTC 0 to DPAD0 and DPAD1 - * by default. + * Configure the superposition processor to pin controller routing. + * Hardcode the assignment, except on Gen1 where we use it to route the + * DU channels to DPAD1. There we route CRTC 0 to DPAD1 if explicitly + * requested, and CRTC 1 in all other cases to avoid cloning CRTC 0 to + * DPAD0 and DPAD1 by default. */ - if (rcdu->dpad1_source == rgrp->index * 2) + if (rcdu->info->gen == 1 && rgrp->index == 0) { + struct rcar_du_crtc_state *rstate; + struct rcar_du_crtc *rcrtc; + + rcrtc = &rcdu->crtcs[0]; + rstate = to_rcar_crtc_state(rcrtc->crtc.state); + + if (rstate->outputs & BIT(RCAR_DU_OUTPUT_DPAD1)) + sp1_to_pin2 = true; + } + + if (sp1_to_pin2) dorcr |= DORCR_PG2D_DS1; else dorcr |= DORCR_PG2T | DORCR_DK2S | DORCR_PG2D_DS2; @@ -333,7 +359,7 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
rcar_du_group_set_dpad_levels(rgrp);
- return rcar_du_set_dpad0_vsp1_routing(rgrp->dev); + rcar_du_group_setup_defr8(rgrp); }
/* ----------------------------------------------------------------------------- @@ -451,20 +477,36 @@ rcar_du_get_new_group_state(struct drm_atomic_state *state, int rcar_du_group_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) { - struct drm_crtc_state *crtc_state; + static const u32 dpad_mask = BIT(RCAR_DU_OUTPUT_DPAD1) + | BIT(RCAR_DU_OUTPUT_DPAD0); + struct drm_crtc_state *old_crtc_state; + struct drm_crtc_state *new_crtc_state; struct drm_crtc *crtc; unsigned int i;
- for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc); + struct rcar_du_crtc_state *old_rcrtc_state; + struct rcar_du_crtc_state *new_rcrtc_state; struct rcar_du_group_state *gstate;
gstate = rcar_du_get_group_state(state, rcrtc->group); if (IS_ERR(gstate)) return PTR_ERR(gstate);
- if (crtc_state->active) + if (new_crtc_state->active) gstate->enabled = true; + + if (!new_crtc_state->active_changed && + !new_crtc_state->connectors_changed) + continue; + + old_rcrtc_state = to_rcar_crtc_state(old_crtc_state); + new_rcrtc_state = to_rcar_crtc_state(new_crtc_state); + + if ((old_rcrtc_state->outputs & dpad_mask) != + (new_rcrtc_state->outputs & dpad_mask)) + gstate->dpad_routing_changed = true; }
return 0; @@ -484,8 +526,14 @@ void rcar_du_group_atomic_setup(struct drm_device *dev, old_state = to_rcar_group_state(old_pstate); new_state = to_rcar_group_state(new_pstate);
- if (!old_state->enabled && new_state->enabled) + if (!new_state->enabled) + continue; + + if (!old_state->enabled) rcar_du_group_setup(rgrp); + + if (!old_state->enabled || new_state->dpad_routing_changed) + rcar_du_group_set_routing(rgrp); } }
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.h b/drivers/gpu/drm/rcar-du/rcar_du_group.h index c6e0ae79ede9..1364e2413095 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_group.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.h @@ -60,11 +60,13 @@ struct rcar_du_group { * struct rcar_du_group_state - Driver-specific group state * @state: base DRM private state * @enabled: true if at least one CRTC in the group is enabled + * @dpad_routing_changed: set if CRTC to DPAD output routing has changed */ struct rcar_du_group_state { struct drm_private_state state;
bool enabled; + bool dpad_routing_changed; };
#define to_rcar_group_state(s) \ @@ -75,7 +77,6 @@ void rcar_du_group_write(struct rcar_du_group *rgrp, u32 reg, u32 data);
void rcar_du_group_start_stop(struct rcar_du_group *rgrp, bool start); void rcar_du_group_restart(struct rcar_du_group *rgrp); -int rcar_du_group_set_routing(struct rcar_du_group *rgrp);
int rcar_du_set_dpad0_vsp1_routing(struct rcar_du_device *rcdu);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 8a0c26939de2..bef07bed0fb4 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -446,14 +446,14 @@ static void rcar_du_atomic_commit_tail(struct drm_atomic_state *old_state) struct rcar_du_device *rcdu = to_rcar_du_device(dev); struct drm_crtc_state *crtc_state; struct drm_crtc *crtc; + unsigned int vspd1_sink = rcdu->vspd1_sink; + unsigned int dpad0_source = rcdu->dpad0_source; unsigned int i;
/* - * Store RGB routing to DPAD0 and DPAD1, the hardware will be configured - * when starting the CRTCs. + * Store RGB routing to DPAD0, the hardware will be configured when + * setting up the groups. */ - rcdu->dpad1_source = -1; - for_each_new_crtc_in_state(old_state, crtc, crtc_state, i) { struct rcar_du_crtc_state *rcrtc_state = to_rcar_crtc_state(crtc_state); @@ -461,9 +461,6 @@ static void rcar_du_atomic_commit_tail(struct drm_atomic_state *old_state)
if (rcrtc_state->outputs & BIT(RCAR_DU_OUTPUT_DPAD0)) rcdu->dpad0_source = rcrtc->index; - - if (rcrtc_state->outputs & BIT(RCAR_DU_OUTPUT_DPAD1)) - rcdu->dpad1_source = rcrtc->index; }
/* Apply the atomic update. */ @@ -474,6 +471,11 @@ static void rcar_du_atomic_commit_tail(struct drm_atomic_state *old_state) rcar_du_crtc_atomic_modeset(dev, old_state); drm_atomic_helper_commit_planes(dev, old_state, DRM_PLANE_COMMIT_ACTIVE_ONLY); + + if (rcdu->vspd1_sink != vspd1_sink || + rcdu->dpad0_source != dpad0_source) + rcar_du_set_dpad0_vsp1_routing(rcdu); + drm_atomic_helper_commit_modeset_enables(dev, old_state);
rcar_du_crtc_atomic_enter_standby(dev, old_state); diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 862197be1e01..2c74c57c73d1 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c @@ -552,14 +552,8 @@ void __rcar_du_plane_setup(struct rcar_du_group *rgrp, if (rcdu->info->gen < 3) rcar_du_plane_setup_scanout(rgrp, state);
- if (state->source == RCAR_DU_PLANE_VSPD1) { - unsigned int vspd1_sink = rgrp->index ? 2 : 0; - - if (rcdu->vspd1_sink != vspd1_sink) { - rcdu->vspd1_sink = vspd1_sink; - rcar_du_set_dpad0_vsp1_routing(rcdu); - } - } + if (state->source == RCAR_DU_PLANE_VSPD1) + rcdu->vspd1_sink = rgrp->index ? 2 : 0; }
int __rcar_du_plane_atomic_check(struct drm_plane *plane,
dri-devel@lists.freedesktop.org
-
Kieran Bingham -
Maxime Ripard