* [PATCH v3] drm: rcar-du: lvds: Get mode from state
@ 2019-12-16 22:30 Laurent Pinchart
2019-12-17 13:48 ` Fabrizio Castro
0 siblings, 1 reply; 2+ messages in thread
From: Laurent Pinchart @ 2019-12-16 22:30 UTC (permalink / raw)
To: dri-devel; +Cc: linux-renesas-soc, Fabrizio Castro, Kieran Bingham
The R-Car LVDS encoder driver implements the bridge .mode_set()
operation for the sole purpose of storing the mode in the LVDS private
data, to be used later when enabling the encoder.
Switch to the bridge .atomic_enable() and .atomic_disable() operations
in order to access the global atomic state, and get the mode from the
state instead. Remove both the unneeded .mode_set() operation and the
display_mode and mode fields storing state data from the rcar_lvds
private structure.
As a side effect we get the CRTC from the state, replace the CRTC
pointer retrieved through the bridge's encoder that shouldn't be used by
atomic drivers.
While at it, clarify a few error messages in rcar_lvds_get_lvds_mode()
and turn them into warnings as they are not fatal.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
Changes since v2:
- Turn a few dev_err() into dev_warn()
- Split rcar_lvds_atomic_enable() to avoid setting the companion's
bridge encoder pointer manually
- Call .atomic_disable() on the companion
Changes since v1:
- Call .atomic_enable() on the companion
- Set companion->encoder in .attach()
---
drivers/gpu/drm/rcar-du/rcar_lvds.c | 140 +++++++++++++++-------------
1 file changed, 74 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 8c6c172bbf2e..284f10d0307f 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -65,9 +65,6 @@ struct rcar_lvds {
struct clk *dotclkin[2]; /* External DU clocks */
} clocks;
- struct drm_display_mode display_mode;
- enum rcar_lvds_mode mode;
-
struct drm_bridge *companion;
bool dual_link;
};
@@ -402,10 +399,53 @@ EXPORT_SYMBOL_GPL(rcar_lvds_clk_disable);
* Bridge
*/
-static void rcar_lvds_enable(struct drm_bridge *bridge)
+static enum rcar_lvds_mode rcar_lvds_get_lvds_mode(struct rcar_lvds *lvds,
+ const struct drm_connector *connector)
+{
+ const struct drm_display_info *info;
+ enum rcar_lvds_mode mode;
+
+ /*
+ * There is no API yet to retrieve LVDS mode from a bridge, only panels
+ * are supported.
+ */
+ if (!lvds->panel)
+ return RCAR_LVDS_MODE_JEIDA;
+
+ info = &connector->display_info;
+ if (!info->num_bus_formats || !info->bus_formats) {
+ dev_warn(lvds->dev,
+ "no LVDS bus format reported, using JEIDA\n");
+ return RCAR_LVDS_MODE_JEIDA;
+ }
+
+ switch (info->bus_formats[0]) {
+ case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
+ case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
+ mode = RCAR_LVDS_MODE_JEIDA;
+ break;
+ case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
+ mode = RCAR_LVDS_MODE_VESA;
+ break;
+ default:
+ dev_warn(lvds->dev,
+ "unsupported LVDS bus format 0x%04x, using JEIDA\n",
+ info->bus_formats[0]);
+ return RCAR_LVDS_MODE_JEIDA;
+ }
+
+ if (info->bus_flags & DRM_BUS_FLAG_DATA_LSB_TO_MSB)
+ mode |= RCAR_LVDS_MODE_MIRROR;
+
+ return mode;
+}
+
+static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge,
+ struct drm_atomic_state *state,
+ struct drm_crtc *crtc,
+ struct drm_connector *connector)
{
struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
- const struct drm_display_mode *mode = &lvds->display_mode;
u32 lvdhcr;
u32 lvdcr0;
int ret;
@@ -416,7 +456,8 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
/* Enable the companion LVDS encoder in dual-link mode. */
if (lvds->dual_link && lvds->companion)
- lvds->companion->funcs->enable(lvds->companion);
+ __rcar_lvds_atomic_enable(lvds->companion, state, crtc,
+ connector);
/*
* Hardcode the channels and control signals routing for now.
@@ -452,18 +493,20 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
* PLL clock configuration on all instances but the companion in
* dual-link mode.
*/
- if (!lvds->dual_link || lvds->companion)
+ if (!lvds->dual_link || lvds->companion) {
+ const struct drm_crtc_state *crtc_state =
+ drm_atomic_get_new_crtc_state(state, crtc);
+ const struct drm_display_mode *mode =
+ &crtc_state->adjusted_mode;
+
lvds->info->pll_setup(lvds, mode->clock * 1000);
+ }
/* Set the LVDS mode and select the input. */
- lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
+ lvdcr0 = rcar_lvds_get_lvds_mode(lvds, connector) << LVDCR0_LVMD_SHIFT;
if (lvds->bridge.encoder) {
- /*
- * FIXME: We should really retrieve the CRTC through the state,
- * but how do we get a state pointer?
- */
- if (drm_crtc_index(lvds->bridge.encoder->crtc) == 2)
+ if (drm_crtc_index(crtc) == 2)
lvdcr0 |= LVDCR0_DUSEL;
}
@@ -520,7 +563,21 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
}
}
-static void rcar_lvds_disable(struct drm_bridge *bridge)
+static void rcar_lvds_atomic_enable(struct drm_bridge *bridge,
+ struct drm_atomic_state *state)
+{
+ struct drm_connector *connector;
+ struct drm_crtc *crtc;
+
+ connector = drm_atomic_get_new_connector_for_encoder(state,
+ bridge->encoder);
+ crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
+
+ __rcar_lvds_atomic_enable(bridge, state, crtc, connector);
+}
+
+static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
+ struct drm_atomic_state *state)
{
struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
@@ -535,7 +592,7 @@ static void rcar_lvds_disable(struct drm_bridge *bridge)
/* Disable the companion LVDS encoder in dual-link mode. */
if (lvds->dual_link && lvds->companion)
- lvds->companion->funcs->disable(lvds->companion);
+ lvds->companion->funcs->atomic_disable(lvds->companion, state);
clk_disable_unprepare(lvds->clocks.mod);
}
@@ -558,54 +615,6 @@ static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
return true;
}
-static void rcar_lvds_get_lvds_mode(struct rcar_lvds *lvds)
-{
- struct drm_display_info *info = &lvds->connector.display_info;
- enum rcar_lvds_mode mode;
-
- /*
- * There is no API yet to retrieve LVDS mode from a bridge, only panels
- * are supported.
- */
- if (!lvds->panel)
- return;
-
- if (!info->num_bus_formats || !info->bus_formats) {
- dev_err(lvds->dev, "no LVDS bus format reported\n");
- return;
- }
-
- switch (info->bus_formats[0]) {
- case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
- case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
- mode = RCAR_LVDS_MODE_JEIDA;
- break;
- case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
- mode = RCAR_LVDS_MODE_VESA;
- break;
- default:
- dev_err(lvds->dev, "unsupported LVDS bus format 0x%04x\n",
- info->bus_formats[0]);
- return;
- }
-
- if (info->bus_flags & DRM_BUS_FLAG_DATA_LSB_TO_MSB)
- mode |= RCAR_LVDS_MODE_MIRROR;
-
- lvds->mode = mode;
-}
-
-static void rcar_lvds_mode_set(struct drm_bridge *bridge,
- const struct drm_display_mode *mode,
- const struct drm_display_mode *adjusted_mode)
-{
- struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
-
- lvds->display_mode = *adjusted_mode;
-
- rcar_lvds_get_lvds_mode(lvds);
-}
-
static int rcar_lvds_attach(struct drm_bridge *bridge)
{
struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
@@ -647,10 +656,9 @@ static void rcar_lvds_detach(struct drm_bridge *bridge)
static const struct drm_bridge_funcs rcar_lvds_bridge_ops = {
.attach = rcar_lvds_attach,
.detach = rcar_lvds_detach,
- .enable = rcar_lvds_enable,
- .disable = rcar_lvds_disable,
+ .atomic_enable = rcar_lvds_atomic_enable,
+ .atomic_disable = rcar_lvds_atomic_disable,
.mode_fixup = rcar_lvds_mode_fixup,
- .mode_set = rcar_lvds_mode_set,
};
bool rcar_lvds_dual_link(struct drm_bridge *bridge)
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 2+ messages in thread
* RE: [PATCH v3] drm: rcar-du: lvds: Get mode from state
2019-12-16 22:30 [PATCH v3] drm: rcar-du: lvds: Get mode from state Laurent Pinchart
@ 2019-12-17 13:48 ` Fabrizio Castro
0 siblings, 0 replies; 2+ messages in thread
From: Fabrizio Castro @ 2019-12-17 13:48 UTC (permalink / raw)
To: Laurent Pinchart, dri-devel; +Cc: linux-renesas-soc, Kieran Bingham
Hi Laurent,
Thank you for your patch!
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Sent: 16 December 2019 22:31
> Subject: [PATCH v3] drm: rcar-du: lvds: Get mode from state
>
> The R-Car LVDS encoder driver implements the bridge .mode_set()
> operation for the sole purpose of storing the mode in the LVDS private
> data, to be used later when enabling the encoder.
>
> Switch to the bridge .atomic_enable() and .atomic_disable() operations
> in order to access the global atomic state, and get the mode from the
> state instead. Remove both the unneeded .mode_set() operation and the
> display_mode and mode fields storing state data from the rcar_lvds
> private structure.
>
> As a side effect we get the CRTC from the state, replace the CRTC
> pointer retrieved through the bridge's encoder that shouldn't be used by
> atomic drivers.
>
> While at it, clarify a few error messages in rcar_lvds_get_lvds_mode()
> and turn them into warnings as they are not fatal.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
And I have tested this on the EK874, therefore
Tested-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> ---
> Changes since v2:
>
> - Turn a few dev_err() into dev_warn()
> - Split rcar_lvds_atomic_enable() to avoid setting the companion's
> bridge encoder pointer manually
> - Call .atomic_disable() on the companion
>
> Changes since v1:
>
> - Call .atomic_enable() on the companion
> - Set companion->encoder in .attach()
> ---
> drivers/gpu/drm/rcar-du/rcar_lvds.c | 140 +++++++++++++++-------------
> 1 file changed, 74 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 8c6c172bbf2e..284f10d0307f 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -65,9 +65,6 @@ struct rcar_lvds {
> struct clk *dotclkin[2]; /* External DU clocks */
> } clocks;
>
> - struct drm_display_mode display_mode;
> - enum rcar_lvds_mode mode;
> -
> struct drm_bridge *companion;
> bool dual_link;
> };
> @@ -402,10 +399,53 @@ EXPORT_SYMBOL_GPL(rcar_lvds_clk_disable);
> * Bridge
> */
>
> -static void rcar_lvds_enable(struct drm_bridge *bridge)
> +static enum rcar_lvds_mode rcar_lvds_get_lvds_mode(struct rcar_lvds *lvds,
> + const struct drm_connector *connector)
> +{
> + const struct drm_display_info *info;
> + enum rcar_lvds_mode mode;
> +
> + /*
> + * There is no API yet to retrieve LVDS mode from a bridge, only panels
> + * are supported.
> + */
> + if (!lvds->panel)
> + return RCAR_LVDS_MODE_JEIDA;
> +
> + info = &connector->display_info;
> + if (!info->num_bus_formats || !info->bus_formats) {
> + dev_warn(lvds->dev,
> + "no LVDS bus format reported, using JEIDA\n");
> + return RCAR_LVDS_MODE_JEIDA;
> + }
> +
> + switch (info->bus_formats[0]) {
> + case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
> + case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
> + mode = RCAR_LVDS_MODE_JEIDA;
> + break;
> + case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
> + mode = RCAR_LVDS_MODE_VESA;
> + break;
> + default:
> + dev_warn(lvds->dev,
> + "unsupported LVDS bus format 0x%04x, using JEIDA\n",
> + info->bus_formats[0]);
> + return RCAR_LVDS_MODE_JEIDA;
> + }
> +
> + if (info->bus_flags & DRM_BUS_FLAG_DATA_LSB_TO_MSB)
> + mode |= RCAR_LVDS_MODE_MIRROR;
> +
> + return mode;
> +}
> +
> +static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge,
> + struct drm_atomic_state *state,
> + struct drm_crtc *crtc,
> + struct drm_connector *connector)
> {
> struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> - const struct drm_display_mode *mode = &lvds->display_mode;
> u32 lvdhcr;
> u32 lvdcr0;
> int ret;
> @@ -416,7 +456,8 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
>
> /* Enable the companion LVDS encoder in dual-link mode. */
> if (lvds->dual_link && lvds->companion)
> - lvds->companion->funcs->enable(lvds->companion);
> + __rcar_lvds_atomic_enable(lvds->companion, state, crtc,
> + connector);
>
> /*
> * Hardcode the channels and control signals routing for now.
> @@ -452,18 +493,20 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> * PLL clock configuration on all instances but the companion in
> * dual-link mode.
> */
> - if (!lvds->dual_link || lvds->companion)
> + if (!lvds->dual_link || lvds->companion) {
> + const struct drm_crtc_state *crtc_state =
> + drm_atomic_get_new_crtc_state(state, crtc);
> + const struct drm_display_mode *mode =
> + &crtc_state->adjusted_mode;
> +
> lvds->info->pll_setup(lvds, mode->clock * 1000);
> + }
>
> /* Set the LVDS mode and select the input. */
> - lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
> + lvdcr0 = rcar_lvds_get_lvds_mode(lvds, connector) << LVDCR0_LVMD_SHIFT;
>
> if (lvds->bridge.encoder) {
> - /*
> - * FIXME: We should really retrieve the CRTC through the state,
> - * but how do we get a state pointer?
> - */
> - if (drm_crtc_index(lvds->bridge.encoder->crtc) == 2)
> + if (drm_crtc_index(crtc) == 2)
> lvdcr0 |= LVDCR0_DUSEL;
> }
>
> @@ -520,7 +563,21 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> }
> }
>
> -static void rcar_lvds_disable(struct drm_bridge *bridge)
> +static void rcar_lvds_atomic_enable(struct drm_bridge *bridge,
> + struct drm_atomic_state *state)
> +{
> + struct drm_connector *connector;
> + struct drm_crtc *crtc;
> +
> + connector = drm_atomic_get_new_connector_for_encoder(state,
> + bridge->encoder);
> + crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
> +
> + __rcar_lvds_atomic_enable(bridge, state, crtc, connector);
> +}
> +
> +static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
> + struct drm_atomic_state *state)
> {
> struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
>
> @@ -535,7 +592,7 @@ static void rcar_lvds_disable(struct drm_bridge *bridge)
>
> /* Disable the companion LVDS encoder in dual-link mode. */
> if (lvds->dual_link && lvds->companion)
> - lvds->companion->funcs->disable(lvds->companion);
> + lvds->companion->funcs->atomic_disable(lvds->companion, state);
>
> clk_disable_unprepare(lvds->clocks.mod);
> }
> @@ -558,54 +615,6 @@ static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
> return true;
> }
>
> -static void rcar_lvds_get_lvds_mode(struct rcar_lvds *lvds)
> -{
> - struct drm_display_info *info = &lvds->connector.display_info;
> - enum rcar_lvds_mode mode;
> -
> - /*
> - * There is no API yet to retrieve LVDS mode from a bridge, only panels
> - * are supported.
> - */
> - if (!lvds->panel)
> - return;
> -
> - if (!info->num_bus_formats || !info->bus_formats) {
> - dev_err(lvds->dev, "no LVDS bus format reported\n");
> - return;
> - }
> -
> - switch (info->bus_formats[0]) {
> - case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
> - case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
> - mode = RCAR_LVDS_MODE_JEIDA;
> - break;
> - case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
> - mode = RCAR_LVDS_MODE_VESA;
> - break;
> - default:
> - dev_err(lvds->dev, "unsupported LVDS bus format 0x%04x\n",
> - info->bus_formats[0]);
> - return;
> - }
> -
> - if (info->bus_flags & DRM_BUS_FLAG_DATA_LSB_TO_MSB)
> - mode |= RCAR_LVDS_MODE_MIRROR;
> -
> - lvds->mode = mode;
> -}
> -
> -static void rcar_lvds_mode_set(struct drm_bridge *bridge,
> - const struct drm_display_mode *mode,
> - const struct drm_display_mode *adjusted_mode)
> -{
> - struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> -
> - lvds->display_mode = *adjusted_mode;
> -
> - rcar_lvds_get_lvds_mode(lvds);
> -}
> -
> static int rcar_lvds_attach(struct drm_bridge *bridge)
> {
> struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> @@ -647,10 +656,9 @@ static void rcar_lvds_detach(struct drm_bridge *bridge)
> static const struct drm_bridge_funcs rcar_lvds_bridge_ops = {
> .attach = rcar_lvds_attach,
> .detach = rcar_lvds_detach,
> - .enable = rcar_lvds_enable,
> - .disable = rcar_lvds_disable,
> + .atomic_enable = rcar_lvds_atomic_enable,
> + .atomic_disable = rcar_lvds_atomic_disable,
> .mode_fixup = rcar_lvds_mode_fixup,
> - .mode_set = rcar_lvds_mode_set,
> };
>
> bool rcar_lvds_dual_link(struct drm_bridge *bridge)
> --
> Regards,
>
> Laurent Pinchart
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-12-17 13:48 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-16 22:30 [PATCH v3] drm: rcar-du: lvds: Get mode from state Laurent Pinchart
2019-12-17 13:48 ` Fabrizio Castro
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).