* [PATCH] drm/bridge: fix LVDS controller bus format
@ 2025-06-18 4:32 Dharma Balasubiramani
2025-06-18 8:41 ` Maxime Ripard
0 siblings, 1 reply; 3+ messages in thread
From: Dharma Balasubiramani @ 2025-06-18 4:32 UTC (permalink / raw)
To: Manikandan Muralidharan, Andrzej Hajda, Neil Armstrong,
Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter
Cc: dri-devel, linux-kernel, Sandeep Sheriker M,
Dharma Balasubiramani
From: Sandeep Sheriker M <sandeep.sheriker@microchip.com>
The current LVDS controller driver is hardcoded to map LVDS lanes to the
JEIDA format. Consequently, connecting an LVDS display that supports the
VESA format results in a distorted display due to the format mismatch.
Query the panel driver and set the appropriate format to resolve the issue.
Signed-off-by: Sandeep Sheriker M <sandeep.sheriker@microchip.com>
Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
drivers/gpu/drm/bridge/microchip-lvds.c | 108 ++++++++++++++++++++++++++++++--
1 file changed, 102 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
index 9f4ff82bc6b4..5e99c01033bb 100644
--- a/drivers/gpu/drm/bridge/microchip-lvds.c
+++ b/drivers/gpu/drm/bridge/microchip-lvds.c
@@ -11,6 +11,7 @@
#include <linux/component.h>
#include <linux/delay.h>
#include <linux/jiffies.h>
+#include <linux/media-bus-format.h>
#include <linux/mfd/syscon.h>
#include <linux/of_graph.h>
#include <linux/pinctrl/devinfo.h>
@@ -41,9 +42,11 @@
/* Bitfields in LVDSC_CFGR (Configuration Register) */
#define LVDSC_CFGR_PIXSIZE_24BITS 0
+#define LVDSC_CFGR_PIXSIZE_18BITS 1
#define LVDSC_CFGR_DEN_POL_HIGH 0
#define LVDSC_CFGR_DC_UNBALANCED 0
#define LVDSC_CFGR_MAPPING_JEIDA BIT(6)
+#define LVDSC_CFGR_MAPPING_VESA 0
/*Bitfields in LVDSC_SR */
#define LVDSC_SR_CS BIT(0)
@@ -58,6 +61,7 @@ struct mchp_lvds {
struct clk *pclk;
struct drm_panel *panel;
struct drm_bridge bridge;
+ struct drm_connector connector;
struct drm_bridge *panel_bridge;
};
@@ -66,6 +70,11 @@ static inline struct mchp_lvds *bridge_to_lvds(struct drm_bridge *bridge)
return container_of(bridge, struct mchp_lvds, bridge);
}
+static inline struct mchp_lvds *drm_connector_to_mchp_lvds(struct drm_connector *connector)
+{
+ return container_of(connector, struct mchp_lvds, connector);
+}
+
static inline u32 lvds_readl(struct mchp_lvds *lvds, u32 offset)
{
return readl_relaxed(lvds->regs + offset);
@@ -79,6 +88,11 @@ static inline void lvds_writel(struct mchp_lvds *lvds, u32 offset, u32 val)
static void lvds_serialiser_on(struct mchp_lvds *lvds)
{
unsigned long timeout = jiffies + msecs_to_jiffies(LVDS_POLL_TIMEOUT_MS);
+ struct drm_connector *connector = &lvds->connector;
+
+ /* default to jeida-24 */
+ u32 bus_formats = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA;
+ u8 map, pix_size;
/* The LVDSC registers can only be written if WPEN is cleared */
lvds_writel(lvds, LVDSC_WPMR, (LVDSC_WPMR_WPKEY_PSSWD &
@@ -93,24 +107,106 @@ static void lvds_serialiser_on(struct mchp_lvds *lvds)
usleep_range(1000, 2000);
}
+ if (connector && connector->display_info.num_bus_formats)
+ bus_formats = connector->display_info.bus_formats[0];
+
/* Configure the LVDSC */
- lvds_writel(lvds, LVDSC_CFGR, (LVDSC_CFGR_MAPPING_JEIDA |
- LVDSC_CFGR_DC_UNBALANCED |
- LVDSC_CFGR_DEN_POL_HIGH |
- LVDSC_CFGR_PIXSIZE_24BITS));
+ switch (bus_formats) {
+ case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
+ map = LVDSC_CFGR_MAPPING_JEIDA;
+ pix_size = LVDSC_CFGR_PIXSIZE_18BITS;
+ break;
+ case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
+ map = LVDSC_CFGR_MAPPING_VESA;
+ pix_size = LVDSC_CFGR_PIXSIZE_24BITS;
+ break;
+ default:
+ map = LVDSC_CFGR_MAPPING_JEIDA;
+ pix_size = LVDSC_CFGR_PIXSIZE_24BITS;
+ break;
+ }
+
+ lvds_writel(lvds, LVDSC_CFGR, (map | LVDSC_CFGR_DC_UNBALANCED |
+ LVDSC_CFGR_DEN_POL_HIGH | pix_size));
/* Enable the LVDS serializer */
lvds_writel(lvds, LVDSC_CR, LVDSC_CR_SER_EN);
}
+static int mchp_lvds_connector_get_modes(struct drm_connector *connector)
+{
+ struct mchp_lvds *lvds = drm_connector_to_mchp_lvds(connector);
+
+ return drm_panel_get_modes(lvds->panel, connector);
+}
+
+static const struct drm_connector_helper_funcs mchp_lvds_connector_helper_funcs = {
+ .get_modes = mchp_lvds_connector_get_modes,
+};
+
+static const struct drm_connector_funcs panel_bridge_connector_funcs = {
+ .reset = drm_atomic_helper_connector_reset,
+ .fill_modes = drm_helper_probe_single_connector_modes,
+ .destroy = drm_connector_cleanup,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
static int mchp_lvds_attach(struct drm_bridge *bridge,
struct drm_encoder *encoder,
enum drm_bridge_attach_flags flags)
{
struct mchp_lvds *lvds = bridge_to_lvds(bridge);
+ struct drm_connector *connector = &lvds->connector;
+ int ret;
+
+ ret = drm_bridge_attach(encoder, lvds->panel_bridge,
+ bridge, flags);
+
+ if (ret < 0)
+ return ret;
+
+ if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
+ return 0;
+
+ if (!encoder) {
+ dev_err(lvds->dev, "Missing encoder\n");
+ return -ENODEV;
+ }
+
+ drm_connector_helper_add(connector,
+ &mchp_lvds_connector_helper_funcs);
+
+ ret = drm_connector_init(bridge->dev, connector,
+ &panel_bridge_connector_funcs,
+ DRM_MODE_CONNECTOR_LVDS);
+ if (ret) {
+ dev_err(lvds->dev, "Failed to initialize connector %d\n", ret);
+ return ret;
+ }
- return drm_bridge_attach(encoder, lvds->panel_bridge,
- bridge, flags);
+ drm_panel_bridge_set_orientation(connector, bridge);
+
+ ret = drm_connector_attach_encoder(&lvds->connector, encoder);
+ if (ret) {
+ dev_err(lvds->dev, "Failed to attach connector to encoder %d\n", ret);
+ drm_connector_cleanup(connector);
+ return ret;
+ }
+
+ if (bridge->dev->registered) {
+ if (connector->funcs->reset)
+ connector->funcs->reset(connector);
+
+ ret = drm_connector_register(connector);
+ if (ret) {
+ dev_err(lvds->dev, "Failed to attach connector to register %d\n", ret);
+ drm_connector_cleanup(connector);
+ return ret;
+ }
+ }
+
+ return 0;
}
static void mchp_lvds_enable(struct drm_bridge *bridge)
---
base-commit: 4325743c7e209ae7845293679a4de94b969f2bef
change-id: 20250618-microchip-lvds-b7151d96094a
Best regards,
--
Dharma Balasubiramani <dharma.b@microchip.com>
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/bridge: fix LVDS controller bus format
2025-06-18 4:32 [PATCH] drm/bridge: fix LVDS controller bus format Dharma Balasubiramani
@ 2025-06-18 8:41 ` Maxime Ripard
2025-06-18 10:42 ` Dharma.B
0 siblings, 1 reply; 3+ messages in thread
From: Maxime Ripard @ 2025-06-18 8:41 UTC (permalink / raw)
To: Dharma Balasubiramani
Cc: Manikandan Muralidharan, Andrzej Hajda, Neil Armstrong,
Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter,
dri-devel, linux-kernel, Sandeep Sheriker M
[-- Attachment #1: Type: text/plain, Size: 7222 bytes --]
Hi,
On Wed, Jun 18, 2025 at 10:02:42AM +0530, Dharma Balasubiramani wrote:
> From: Sandeep Sheriker M <sandeep.sheriker@microchip.com>
>
> The current LVDS controller driver is hardcoded to map LVDS lanes to the
> JEIDA format. Consequently, connecting an LVDS display that supports the
> VESA format results in a distorted display due to the format mismatch.
>
> Query the panel driver and set the appropriate format to resolve the issue.
>
> Signed-off-by: Sandeep Sheriker M <sandeep.sheriker@microchip.com>
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
It looks like there's a bit of context missing to explain why you needed
to do it that way. See below.
> ---
> drivers/gpu/drm/bridge/microchip-lvds.c | 108 ++++++++++++++++++++++++++++++--
> 1 file changed, 102 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
> index 9f4ff82bc6b4..5e99c01033bb 100644
> --- a/drivers/gpu/drm/bridge/microchip-lvds.c
> +++ b/drivers/gpu/drm/bridge/microchip-lvds.c
> @@ -11,6 +11,7 @@
> #include <linux/component.h>
> #include <linux/delay.h>
> #include <linux/jiffies.h>
> +#include <linux/media-bus-format.h>
> #include <linux/mfd/syscon.h>
> #include <linux/of_graph.h>
> #include <linux/pinctrl/devinfo.h>
> @@ -41,9 +42,11 @@
>
> /* Bitfields in LVDSC_CFGR (Configuration Register) */
> #define LVDSC_CFGR_PIXSIZE_24BITS 0
> +#define LVDSC_CFGR_PIXSIZE_18BITS 1
> #define LVDSC_CFGR_DEN_POL_HIGH 0
> #define LVDSC_CFGR_DC_UNBALANCED 0
> #define LVDSC_CFGR_MAPPING_JEIDA BIT(6)
> +#define LVDSC_CFGR_MAPPING_VESA 0
>
> /*Bitfields in LVDSC_SR */
> #define LVDSC_SR_CS BIT(0)
> @@ -58,6 +61,7 @@ struct mchp_lvds {
> struct clk *pclk;
> struct drm_panel *panel;
> struct drm_bridge bridge;
> + struct drm_connector connector;
> struct drm_bridge *panel_bridge;
> };
>
> @@ -66,6 +70,11 @@ static inline struct mchp_lvds *bridge_to_lvds(struct drm_bridge *bridge)
> return container_of(bridge, struct mchp_lvds, bridge);
> }
>
> +static inline struct mchp_lvds *drm_connector_to_mchp_lvds(struct drm_connector *connector)
> +{
> + return container_of(connector, struct mchp_lvds, connector);
> +}
> +
> static inline u32 lvds_readl(struct mchp_lvds *lvds, u32 offset)
> {
> return readl_relaxed(lvds->regs + offset);
> @@ -79,6 +88,11 @@ static inline void lvds_writel(struct mchp_lvds *lvds, u32 offset, u32 val)
> static void lvds_serialiser_on(struct mchp_lvds *lvds)
> {
> unsigned long timeout = jiffies + msecs_to_jiffies(LVDS_POLL_TIMEOUT_MS);
> + struct drm_connector *connector = &lvds->connector;
How does that work if the bridge was attached with NO_CONNECTOR? Would
the structure be uninitialized?
> +
> + /* default to jeida-24 */
> + u32 bus_formats = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA;
> + u8 map, pix_size;
>
> /* The LVDSC registers can only be written if WPEN is cleared */
> lvds_writel(lvds, LVDSC_WPMR, (LVDSC_WPMR_WPKEY_PSSWD &
> @@ -93,24 +107,106 @@ static void lvds_serialiser_on(struct mchp_lvds *lvds)
> usleep_range(1000, 2000);
> }
>
> + if (connector && connector->display_info.num_bus_formats)
> + bus_formats = connector->display_info.bus_formats[0];
> +
> /* Configure the LVDSC */
> - lvds_writel(lvds, LVDSC_CFGR, (LVDSC_CFGR_MAPPING_JEIDA |
> - LVDSC_CFGR_DC_UNBALANCED |
> - LVDSC_CFGR_DEN_POL_HIGH |
> - LVDSC_CFGR_PIXSIZE_24BITS));
> + switch (bus_formats) {
> + case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
> + map = LVDSC_CFGR_MAPPING_JEIDA;
> + pix_size = LVDSC_CFGR_PIXSIZE_18BITS;
> + break;
> + case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
> + map = LVDSC_CFGR_MAPPING_VESA;
> + pix_size = LVDSC_CFGR_PIXSIZE_24BITS;
> + break;
> + default:
> + map = LVDSC_CFGR_MAPPING_JEIDA;
> + pix_size = LVDSC_CFGR_PIXSIZE_24BITS;
> + break;
> + }
> +
> + lvds_writel(lvds, LVDSC_CFGR, (map | LVDSC_CFGR_DC_UNBALANCED |
> + LVDSC_CFGR_DEN_POL_HIGH | pix_size));
>
> /* Enable the LVDS serializer */
> lvds_writel(lvds, LVDSC_CR, LVDSC_CR_SER_EN);
> }
Aside from the bit above, that part looks fine to me.
>
> +static int mchp_lvds_connector_get_modes(struct drm_connector *connector)
> +{
> + struct mchp_lvds *lvds = drm_connector_to_mchp_lvds(connector);
> +
> + return drm_panel_get_modes(lvds->panel, connector);
> +}
> +
> +static const struct drm_connector_helper_funcs mchp_lvds_connector_helper_funcs = {
> + .get_modes = mchp_lvds_connector_get_modes,
> +};
> +
> +static const struct drm_connector_funcs panel_bridge_connector_funcs = {
> + .reset = drm_atomic_helper_connector_reset,
> + .fill_modes = drm_helper_probe_single_connector_modes,
> + .destroy = drm_connector_cleanup,
> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> static int mchp_lvds_attach(struct drm_bridge *bridge,
> struct drm_encoder *encoder,
> enum drm_bridge_attach_flags flags)
> {
> struct mchp_lvds *lvds = bridge_to_lvds(bridge);
> + struct drm_connector *connector = &lvds->connector;
> + int ret;
> +
> + ret = drm_bridge_attach(encoder, lvds->panel_bridge,
> + bridge, flags);
> +
> + if (ret < 0)
> + return ret;
> +
> + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
> + return 0;
> +
> + if (!encoder) {
> + dev_err(lvds->dev, "Missing encoder\n");
> + return -ENODEV;
> + }
> +
> + drm_connector_helper_add(connector,
> + &mchp_lvds_connector_helper_funcs);
> +
> + ret = drm_connector_init(bridge->dev, connector,
> + &panel_bridge_connector_funcs,
> + DRM_MODE_CONNECTOR_LVDS);
> + if (ret) {
> + dev_err(lvds->dev, "Failed to initialize connector %d\n", ret);
> + return ret;
> + }
>
> - return drm_bridge_attach(encoder, lvds->panel_bridge,
> - bridge, flags);
> + drm_panel_bridge_set_orientation(connector, bridge);
> +
> + ret = drm_connector_attach_encoder(&lvds->connector, encoder);
> + if (ret) {
> + dev_err(lvds->dev, "Failed to attach connector to encoder %d\n", ret);
> + drm_connector_cleanup(connector);
> + return ret;
> + }
> +
> + if (bridge->dev->registered) {
> + if (connector->funcs->reset)
> + connector->funcs->reset(connector);
> +
> + ret = drm_connector_register(connector);
> + if (ret) {
> + dev_err(lvds->dev, "Failed to attach connector to register %d\n", ret);
> + drm_connector_cleanup(connector);
> + return ret;
> + }
> + }
> +
> + return 0;
However, this is the part I don't really get. AFAIU, you create a
connector, for the sole purpose of calling the panel get_modes. But the
panel bridge you already using is calling that function already. So
there must be something more.
Did you create the connector only to be able to access it in
lvds_serialiser_on and thus retrieve the bus_formats? If so, you should
convert enable / disable to atomic_enable / atomic_disable, pass
drm_atomic_state to lvds_serialiser_on, and then call
drm_atomic_get_new_connector_for_encoder(bridge->encoder).
Maximee
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/bridge: fix LVDS controller bus format
2025-06-18 8:41 ` Maxime Ripard
@ 2025-06-18 10:42 ` Dharma.B
0 siblings, 0 replies; 3+ messages in thread
From: Dharma.B @ 2025-06-18 10:42 UTC (permalink / raw)
To: mripard
Cc: Manikandan.M, andrzej.hajda, neil.armstrong, rfoss,
Laurent.pinchart, jonas, jernej.skrabec, maarten.lankhorst,
tzimmermann, airlied, simona, dri-devel, linux-kernel,
Sandeep.Sheriker
Hi Maxime,
On 18/06/25 2:11 pm, Maxime Ripard wrote:
> Hi,
>
> On Wed, Jun 18, 2025 at 10:02:42AM +0530, Dharma Balasubiramani wrote:
>> From: Sandeep Sheriker M <sandeep.sheriker@microchip.com>
>>
>> The current LVDS controller driver is hardcoded to map LVDS lanes to the
>> JEIDA format. Consequently, connecting an LVDS display that supports the
>> VESA format results in a distorted display due to the format mismatch.
>>
>> Query the panel driver and set the appropriate format to resolve the issue.
>>
>> Signed-off-by: Sandeep Sheriker M <sandeep.sheriker@microchip.com>
>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
>
> It looks like there's a bit of context missing to explain why you needed
> to do it that way. See below.
>
>> ---
>> drivers/gpu/drm/bridge/microchip-lvds.c | 108 ++++++++++++++++++++++++++++++--
>> 1 file changed, 102 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
>> index 9f4ff82bc6b4..5e99c01033bb 100644
>> --- a/drivers/gpu/drm/bridge/microchip-lvds.c
>> +++ b/drivers/gpu/drm/bridge/microchip-lvds.c
>> @@ -11,6 +11,7 @@
>> #include <linux/component.h>
>> #include <linux/delay.h>
>> #include <linux/jiffies.h>
>> +#include <linux/media-bus-format.h>
>> #include <linux/mfd/syscon.h>
>> #include <linux/of_graph.h>
>> #include <linux/pinctrl/devinfo.h>
>> @@ -41,9 +42,11 @@
>>
>> /* Bitfields in LVDSC_CFGR (Configuration Register) */
>> #define LVDSC_CFGR_PIXSIZE_24BITS 0
>> +#define LVDSC_CFGR_PIXSIZE_18BITS 1
>> #define LVDSC_CFGR_DEN_POL_HIGH 0
>> #define LVDSC_CFGR_DC_UNBALANCED 0
>> #define LVDSC_CFGR_MAPPING_JEIDA BIT(6)
>> +#define LVDSC_CFGR_MAPPING_VESA 0
>>
>> /*Bitfields in LVDSC_SR */
>> #define LVDSC_SR_CS BIT(0)
>> @@ -58,6 +61,7 @@ struct mchp_lvds {
>> struct clk *pclk;
>> struct drm_panel *panel;
>> struct drm_bridge bridge;
>> + struct drm_connector connector;
>> struct drm_bridge *panel_bridge;
>> };
>>
>> @@ -66,6 +70,11 @@ static inline struct mchp_lvds *bridge_to_lvds(struct drm_bridge *bridge)
>> return container_of(bridge, struct mchp_lvds, bridge);
>> }
>>
>> +static inline struct mchp_lvds *drm_connector_to_mchp_lvds(struct drm_connector *connector)
>> +{
>> + return container_of(connector, struct mchp_lvds, connector);
>> +}
>> +
>> static inline u32 lvds_readl(struct mchp_lvds *lvds, u32 offset)
>> {
>> return readl_relaxed(lvds->regs + offset);
>> @@ -79,6 +88,11 @@ static inline void lvds_writel(struct mchp_lvds *lvds, u32 offset, u32 val)
>> static void lvds_serialiser_on(struct mchp_lvds *lvds)
>> {
>> unsigned long timeout = jiffies + msecs_to_jiffies(LVDS_POLL_TIMEOUT_MS);
>> + struct drm_connector *connector = &lvds->connector;
>
> How does that work if the bridge was attached with NO_CONNECTOR? Would
> the structure be uninitialized?
Thanks for pointing it out, I will perform the changes as you suggested
below.
>
>> +
>> + /* default to jeida-24 */
>> + u32 bus_formats = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA;
>> + u8 map, pix_size;
>>
>> /* The LVDSC registers can only be written if WPEN is cleared */
>> lvds_writel(lvds, LVDSC_WPMR, (LVDSC_WPMR_WPKEY_PSSWD &
>> @@ -93,24 +107,106 @@ static void lvds_serialiser_on(struct mchp_lvds *lvds)
>> usleep_range(1000, 2000);
>> }
>>
>> + if (connector && connector->display_info.num_bus_formats)
>> + bus_formats = connector->display_info.bus_formats[0];
>> +
>> /* Configure the LVDSC */
>> - lvds_writel(lvds, LVDSC_CFGR, (LVDSC_CFGR_MAPPING_JEIDA |
>> - LVDSC_CFGR_DC_UNBALANCED |
>> - LVDSC_CFGR_DEN_POL_HIGH |
>> - LVDSC_CFGR_PIXSIZE_24BITS));
>> + switch (bus_formats) {
>> + case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
>> + map = LVDSC_CFGR_MAPPING_JEIDA;
>> + pix_size = LVDSC_CFGR_PIXSIZE_18BITS;
>> + break;
>> + case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
>> + map = LVDSC_CFGR_MAPPING_VESA;
>> + pix_size = LVDSC_CFGR_PIXSIZE_24BITS;
>> + break;
>> + default:
>> + map = LVDSC_CFGR_MAPPING_JEIDA;
>> + pix_size = LVDSC_CFGR_PIXSIZE_24BITS;
>> + break;
>> + }
>> +
>> + lvds_writel(lvds, LVDSC_CFGR, (map | LVDSC_CFGR_DC_UNBALANCED |
>> + LVDSC_CFGR_DEN_POL_HIGH | pix_size));
>>
>> /* Enable the LVDS serializer */
>> lvds_writel(lvds, LVDSC_CR, LVDSC_CR_SER_EN);
>> }
>
> Aside from the bit above, that part looks fine to me.
>
>>
>> +static int mchp_lvds_connector_get_modes(struct drm_connector *connector)
>> +{
>> + struct mchp_lvds *lvds = drm_connector_to_mchp_lvds(connector);
>> +
>> + return drm_panel_get_modes(lvds->panel, connector);
>> +}
>> +
>> +static const struct drm_connector_helper_funcs mchp_lvds_connector_helper_funcs = {
>> + .get_modes = mchp_lvds_connector_get_modes,
>> +};
>> +
>> +static const struct drm_connector_funcs panel_bridge_connector_funcs = {
>> + .reset = drm_atomic_helper_connector_reset,
>> + .fill_modes = drm_helper_probe_single_connector_modes,
>> + .destroy = drm_connector_cleanup,
>> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> +};
>> +
>> static int mchp_lvds_attach(struct drm_bridge *bridge,
>> struct drm_encoder *encoder,
>> enum drm_bridge_attach_flags flags)
>> {
>> struct mchp_lvds *lvds = bridge_to_lvds(bridge);
>> + struct drm_connector *connector = &lvds->connector;
>> + int ret;
>> +
>> + ret = drm_bridge_attach(encoder, lvds->panel_bridge,
>> + bridge, flags);
>> +
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
>> + return 0;
>> +
>> + if (!encoder) {
>> + dev_err(lvds->dev, "Missing encoder\n");
>> + return -ENODEV;
>> + }
>> +
>> + drm_connector_helper_add(connector,
>> + &mchp_lvds_connector_helper_funcs);
>> +
>> + ret = drm_connector_init(bridge->dev, connector,
>> + &panel_bridge_connector_funcs,
>> + DRM_MODE_CONNECTOR_LVDS);
>> + if (ret) {
>> + dev_err(lvds->dev, "Failed to initialize connector %d\n", ret);
>> + return ret;
>> + }
>>
>> - return drm_bridge_attach(encoder, lvds->panel_bridge,
>> - bridge, flags);
>> + drm_panel_bridge_set_orientation(connector, bridge);
>> +
>> + ret = drm_connector_attach_encoder(&lvds->connector, encoder);
>> + if (ret) {
>> + dev_err(lvds->dev, "Failed to attach connector to encoder %d\n", ret);
>> + drm_connector_cleanup(connector);
>> + return ret;
>> + }
>> +
>> + if (bridge->dev->registered) {
>> + if (connector->funcs->reset)
>> + connector->funcs->reset(connector);
>> +
>> + ret = drm_connector_register(connector);
>> + if (ret) {
>> + dev_err(lvds->dev, "Failed to attach connector to register %d\n", ret);
>> + drm_connector_cleanup(connector);
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>
> However, this is the part I don't really get. AFAIU, you create a
> connector, for the sole purpose of calling the panel get_modes. But the
> panel bridge you already using is calling that function already. So
> there must be something more.
>
> Did you create the connector only to be able to access it in
> lvds_serialiser_on and thus retrieve the bus_formats? If so, you should
> convert enable / disable to atomic_enable / atomic_disable, pass
> drm_atomic_state to lvds_serialiser_on, and then call
> drm_atomic_get_new_connector_for_encoder(bridge->encoder).
Sure, I will drop enable / disable and add
.atomic_pre_enable,.atomic_enable,.atomic_disable,.atomic_post_disable
I will get the bus_format as you suggested
"
connector = drm_atomic_get_new_connector_for_encoder(state,
bridge->encoder);
if (connector && connector->display_info.num_bus_formats)
bus_format = connector->display_info.bus_formats[0];
lvds_serialiser_on(lvds, bus_format);
"
and will drop the rest in v2.
Thanks.
>
> Maximee
--
With Best Regards,
Dharma B.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-06-18 10:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-18 4:32 [PATCH] drm/bridge: fix LVDS controller bus format Dharma Balasubiramani
2025-06-18 8:41 ` Maxime Ripard
2025-06-18 10:42 ` Dharma.B
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).