On 1/23/23 16:12, Jagan Teki wrote:
DSI host registration, attach and detach operations are quite
different for the component and bridge-based DRM drivers.
Supporting generic bridge driver to use both component and bridge
based DRM drivers can be tricky and would require additional host
related operation hooks.
Add host operation hooks for registering and unregistering Exynos
and generic drivers, where Exynos hooks are used in existing Exynos
component based DRM drivers and generic hooks are used in i.MX8M
bridge based DRM drivers.
Add host attach and detach operation hooks for Exynos component
DRM drivers and those get invoked while DSI core host attach and
detach gets called.
Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
---
Changes for v11:
- none
Changes for v10:
- split from previous series patch
"drm: bridge: Generalize Exynos-DSI driver into a Samsung DSIM bridge"
drivers/gpu/drm/exynos/exynos_drm_dsi.c | 179 ++++++++++++++++++------
1 file changed, 140 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 7afbbe30d1d3..fc7f00ab01b4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -250,6 +250,8 @@ struct exynos_dsi_transfer {
u16 rx_done;
};
+struct exynos_dsi;
Is this forward declaration really necessary ? Can't the structures
below be reordered to get rid of this ?
#define DSIM_STATE_ENABLED BIT(0)
#define DSIM_STATE_INITIALIZED BIT(1)
#define DSIM_STATE_CMD_LPM BIT(2)
@@ -281,12 +283,19 @@ struct exynos_dsi_driver_data {
const unsigned int *reg_values;
};
+struct exynos_dsim_host_ops {
+ int (*register_host)(struct exynos_dsi *dsim);
+ void (*unregister_host)(struct exynos_dsi *dsim);
+ int (*attach)(struct exynos_dsi *dsim, struct mipi_dsi_device *device);
+ int (*detach)(struct exynos_dsi *dsim, struct mipi_dsi_device *device);
+};
+
struct exynos_dsi_plat_data {
enum exynos_dsi_type hw_type;
+ const struct exynos_dsim_host_ops *host_ops;
};
struct exynos_dsi {
- struct drm_encoder encoder;
struct mipi_dsi_host dsi_host;
struct drm_bridge bridge;
struct drm_bridge *out_bridge;
@@ -316,6 +325,12 @@ struct exynos_dsi {
const struct exynos_dsi_driver_data *driver_data;
const struct exynos_dsi_plat_data *plat_data;
+
+ void *priv;
+};
+
+struct exynos_dsi_enc {
+ struct drm_encoder encoder;
};
#define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host)
@@ -1319,10 +1334,11 @@ static irqreturn_t exynos_dsi_irq(int irq, void *dev_id)
static irqreturn_t exynos_dsi_te_irq_handler(int irq, void *dev_id)
{
- struct exynos_dsi *dsi = (struct exynos_dsi *)dev_id;
+ struct exynos_dsi *dsim = (struct exynos_dsi *)dev_id;
Is the rename really needed ?
+ struct exynos_dsi_enc *dsi = dsim->priv;
Call this variable something else , like dsi_enc , and you shouldn't
need the rename above ...
struct drm_encoder *encoder = &dsi->encoder;
- if (dsi->state & DSIM_STATE_VIDOUT_AVAILABLE)
+ if (dsim->state & DSIM_STATE_VIDOUT_AVAILABLE)
... and the rename here .
exynos_drm_crtc_te_handler(encoder->crtc);
return IRQ_HANDLED;
[...]
static void exynos_dsi_unbind(struct device *dev, struct device *master,
void *data)
{
- struct exynos_dsi *dsi = dev_get_drvdata(dev);
+ struct exynos_dsi *dsim = dev_get_drvdata(dev);
Please avoid the variable renames globally, that should simplify this
patch and remove unrelated changes.
- exynos_dsi_atomic_disable(&dsi->bridge, NULL);
+ dsim->bridge.funcs->atomic_disable(&dsim->bridge, NULL);
- mipi_dsi_host_unregister(&dsi->dsi_host);
+ mipi_dsi_host_unregister(&dsim->dsi_host);
}
[...]
With that fixed:
Reviewed-by: Marek Vasut <marex@xxxxxxx>
