* [PATCH 2/3] mfd: atmel-hlcdc: fetch LVDS PLL clock for LVDS display
2024-11-21 9:23 [PATCH 1/3] drm: atmel-hlcdc: add support for LVDS encoder type Manikandan Muralidharan
@ 2024-11-21 9:23 ` Manikandan Muralidharan
2025-02-10 17:25 ` Lee Jones
2024-11-21 9:23 ` [PATCH 3/3] drm: atmel-hlcdc: set LVDS PLL clock rate for LVDS Displays Manikandan Muralidharan
2025-01-28 5:03 ` [PATCH 1/3] drm: atmel-hlcdc: add support for LVDS encoder type Manikandan.M
2 siblings, 1 reply; 6+ messages in thread
From: Manikandan Muralidharan @ 2024-11-21 9:23 UTC (permalink / raw)
To: sam, bbrezillon, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, nicolas.ferre, alexandre.belloni, claudiu.beznea, lee,
dri-devel, linux-arm-kernel, linux-kernel
Cc: manikandan.m, Dharma Balasubiramani
The XLCDC IP supports DSI, parallel RGB and LVDS Display.
sys_clk(Generic clock) is used for DSI and Parallel RGB displays;
And LVDS PLL is used with LVDS displays.
obtain anyone of the clocks for the LCD to operate
Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
drivers/mfd/atmel-hlcdc.c | 16 ++++++++++++++--
include/linux/mfd/atmel-hlcdc.h | 1 +
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/mfd/atmel-hlcdc.c b/drivers/mfd/atmel-hlcdc.c
index 4c4e35d404f3..60b0b766459e 100644
--- a/drivers/mfd/atmel-hlcdc.c
+++ b/drivers/mfd/atmel-hlcdc.c
@@ -108,10 +108,22 @@ static int atmel_hlcdc_probe(struct platform_device *pdev)
return PTR_ERR(hlcdc->periph_clk);
}
+ /*
+ * Obtain one of the main clocks (GCK / LVDS PLL) required by the
+ * LCD to function,
+ * GCK for Parallel RGB and MIPI displays;
+ * LVDS PLL for LVDS displays.
+ */
+ hlcdc->sys_clk = NULL;
+ hlcdc->lvds_pll_clk = NULL;
hlcdc->sys_clk = devm_clk_get(dev, "sys_clk");
if (IS_ERR(hlcdc->sys_clk)) {
- dev_err(dev, "failed to get system clock\n");
- return PTR_ERR(hlcdc->sys_clk);
+ dev_dbg(dev, "failed to get system clock\n");
+ hlcdc->lvds_pll_clk = devm_clk_get(dev, "lvds_pll_clk");
+ if (IS_ERR(hlcdc->lvds_pll_clk)) {
+ dev_err(dev, "failed to get LVDS PLL clock\n");
+ return PTR_ERR(hlcdc->lvds_pll_clk);
+ }
}
hlcdc->slow_clk = devm_clk_get(dev, "slow_clk");
diff --git a/include/linux/mfd/atmel-hlcdc.h b/include/linux/mfd/atmel-hlcdc.h
index 80d675a03b39..07c2081867fd 100644
--- a/include/linux/mfd/atmel-hlcdc.h
+++ b/include/linux/mfd/atmel-hlcdc.h
@@ -75,6 +75,7 @@
*/
struct atmel_hlcdc {
struct regmap *regmap;
+ struct clk *lvds_pll_clk;
struct clk *periph_clk;
struct clk *sys_clk;
struct clk *slow_clk;
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] mfd: atmel-hlcdc: fetch LVDS PLL clock for LVDS display
2024-11-21 9:23 ` [PATCH 2/3] mfd: atmel-hlcdc: fetch LVDS PLL clock for LVDS display Manikandan Muralidharan
@ 2025-02-10 17:25 ` Lee Jones
0 siblings, 0 replies; 6+ messages in thread
From: Lee Jones @ 2025-02-10 17:25 UTC (permalink / raw)
To: Manikandan Muralidharan
Cc: simona, alexandre.belloni, linux-kernel, bbrezillon, sam,
maarten.lankhorst, mripard, claudiu.beznea, Dharma Balasubiramani,
dri-devel, tzimmermann, airlied, linux-arm-kernel
On Thu, 21 Nov 2024, Manikandan Muralidharan wrote:
> The XLCDC IP supports DSI, parallel RGB and LVDS Display.
> sys_clk(Generic clock) is used for DSI and Parallel RGB displays;
> And LVDS PLL is used with LVDS displays.
> obtain anyone of the clocks for the LCD to operate
Maybe say something about this being a fall-back strategy.
>
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> ---
> drivers/mfd/atmel-hlcdc.c | 16 ++++++++++++++--
> include/linux/mfd/atmel-hlcdc.h | 1 +
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mfd/atmel-hlcdc.c b/drivers/mfd/atmel-hlcdc.c
> index 4c4e35d404f3..60b0b766459e 100644
> --- a/drivers/mfd/atmel-hlcdc.c
> +++ b/drivers/mfd/atmel-hlcdc.c
> @@ -108,10 +108,22 @@ static int atmel_hlcdc_probe(struct platform_device *pdev)
> return PTR_ERR(hlcdc->periph_clk);
> }
>
> + /*
> + * Obtain one of the main clocks (GCK / LVDS PLL) required by the
> + * LCD to function,
> + * GCK for Parallel RGB and MIPI displays;
> + * LVDS PLL for LVDS displays.
> + */
> + hlcdc->sys_clk = NULL;
> + hlcdc->lvds_pll_clk = NULL;
Since, devm_kzalloc() is used to allocate this memory space, these are
already NULL.
> hlcdc->sys_clk = devm_clk_get(dev, "sys_clk");
> if (IS_ERR(hlcdc->sys_clk)) {
> - dev_err(dev, "failed to get system clock\n");
> - return PTR_ERR(hlcdc->sys_clk);
> + dev_dbg(dev, "failed to get system clock\n");
This should be a dev_warn().
May be add some more information.
"failed to get system clock, trying xyz instead"
'\n' here.
> + hlcdc->lvds_pll_clk = devm_clk_get(dev, "lvds_pll_clk");
> + if (IS_ERR(hlcdc->lvds_pll_clk)) {
> + dev_err(dev, "failed to get LVDS PLL clock\n");
> + return PTR_ERR(hlcdc->lvds_pll_clk);
> + }
> }
>
> hlcdc->slow_clk = devm_clk_get(dev, "slow_clk");
> diff --git a/include/linux/mfd/atmel-hlcdc.h b/include/linux/mfd/atmel-hlcdc.h
> index 80d675a03b39..07c2081867fd 100644
> --- a/include/linux/mfd/atmel-hlcdc.h
> +++ b/include/linux/mfd/atmel-hlcdc.h
> @@ -75,6 +75,7 @@
> */
> struct atmel_hlcdc {
> struct regmap *regmap;
> + struct clk *lvds_pll_clk;
> struct clk *periph_clk;
> struct clk *sys_clk;
> struct clk *slow_clk;
> --
> 2.25.1
>
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] mfd: atmel-hlcdc: fetch LVDS PLL clock for LVDS display
@ 2025-02-10 17:25 ` Lee Jones
0 siblings, 0 replies; 6+ messages in thread
From: Lee Jones @ 2025-02-10 17:25 UTC (permalink / raw)
To: Manikandan Muralidharan
Cc: sam, bbrezillon, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, nicolas.ferre, alexandre.belloni, claudiu.beznea,
dri-devel, linux-arm-kernel, linux-kernel, Dharma Balasubiramani
On Thu, 21 Nov 2024, Manikandan Muralidharan wrote:
> The XLCDC IP supports DSI, parallel RGB and LVDS Display.
> sys_clk(Generic clock) is used for DSI and Parallel RGB displays;
> And LVDS PLL is used with LVDS displays.
> obtain anyone of the clocks for the LCD to operate
Maybe say something about this being a fall-back strategy.
>
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> ---
> drivers/mfd/atmel-hlcdc.c | 16 ++++++++++++++--
> include/linux/mfd/atmel-hlcdc.h | 1 +
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mfd/atmel-hlcdc.c b/drivers/mfd/atmel-hlcdc.c
> index 4c4e35d404f3..60b0b766459e 100644
> --- a/drivers/mfd/atmel-hlcdc.c
> +++ b/drivers/mfd/atmel-hlcdc.c
> @@ -108,10 +108,22 @@ static int atmel_hlcdc_probe(struct platform_device *pdev)
> return PTR_ERR(hlcdc->periph_clk);
> }
>
> + /*
> + * Obtain one of the main clocks (GCK / LVDS PLL) required by the
> + * LCD to function,
> + * GCK for Parallel RGB and MIPI displays;
> + * LVDS PLL for LVDS displays.
> + */
> + hlcdc->sys_clk = NULL;
> + hlcdc->lvds_pll_clk = NULL;
Since, devm_kzalloc() is used to allocate this memory space, these are
already NULL.
> hlcdc->sys_clk = devm_clk_get(dev, "sys_clk");
> if (IS_ERR(hlcdc->sys_clk)) {
> - dev_err(dev, "failed to get system clock\n");
> - return PTR_ERR(hlcdc->sys_clk);
> + dev_dbg(dev, "failed to get system clock\n");
This should be a dev_warn().
May be add some more information.
"failed to get system clock, trying xyz instead"
'\n' here.
> + hlcdc->lvds_pll_clk = devm_clk_get(dev, "lvds_pll_clk");
> + if (IS_ERR(hlcdc->lvds_pll_clk)) {
> + dev_err(dev, "failed to get LVDS PLL clock\n");
> + return PTR_ERR(hlcdc->lvds_pll_clk);
> + }
> }
>
> hlcdc->slow_clk = devm_clk_get(dev, "slow_clk");
> diff --git a/include/linux/mfd/atmel-hlcdc.h b/include/linux/mfd/atmel-hlcdc.h
> index 80d675a03b39..07c2081867fd 100644
> --- a/include/linux/mfd/atmel-hlcdc.h
> +++ b/include/linux/mfd/atmel-hlcdc.h
> @@ -75,6 +75,7 @@
> */
> struct atmel_hlcdc {
> struct regmap *regmap;
> + struct clk *lvds_pll_clk;
> struct clk *periph_clk;
> struct clk *sys_clk;
> struct clk *slow_clk;
> --
> 2.25.1
>
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] drm: atmel-hlcdc: set LVDS PLL clock rate for LVDS Displays
2024-11-21 9:23 [PATCH 1/3] drm: atmel-hlcdc: add support for LVDS encoder type Manikandan Muralidharan
2024-11-21 9:23 ` [PATCH 2/3] mfd: atmel-hlcdc: fetch LVDS PLL clock for LVDS display Manikandan Muralidharan
@ 2024-11-21 9:23 ` Manikandan Muralidharan
2025-01-28 5:03 ` [PATCH 1/3] drm: atmel-hlcdc: add support for LVDS encoder type Manikandan.M
2 siblings, 0 replies; 6+ messages in thread
From: Manikandan Muralidharan @ 2024-11-21 9:23 UTC (permalink / raw)
To: sam, bbrezillon, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, nicolas.ferre, alexandre.belloni, claudiu.beznea, lee,
dri-devel, linux-arm-kernel, linux-kernel
Cc: manikandan.m, Dharma Balasubiramani
From: Dharma Balasubiramani <dharma.b@microchip.com>
The LVDS PLL clock is 7x the Panel Pixel clock.
When using LVDS displays, the LVDS PLL clock rate is set using the
panel pixel clock, this skips the usage of 'assigned-clock-rates'
DT property for lvds_pll_clk clock for LCD node.
Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
.../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 48 ++++++++++++++++---
1 file changed, 42 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 0e709047369a..d11040d5cc5f 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -99,9 +99,15 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
drm_connector_list_iter_end(&iter);
}
- ret = clk_prepare_enable(crtc->dc->hlcdc->sys_clk);
- if (ret)
- return;
+ if (crtc->dc->hlcdc->lvds_pll_clk) {
+ ret = clk_prepare_enable(crtc->dc->hlcdc->lvds_pll_clk);
+ if (ret)
+ return;
+ } else {
+ ret = clk_prepare_enable(crtc->dc->hlcdc->sys_clk);
+ if (ret)
+ return;
+ }
vm.vfront_porch = adj->crtc_vsync_start - adj->crtc_vdisplay;
vm.vback_porch = adj->crtc_vtotal - adj->crtc_vsync_end;
@@ -186,7 +192,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
ATMEL_XLCDC_DPI : ATMEL_HLCDC_MODE_MASK),
cfg);
- clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
+ if (crtc->dc->hlcdc->lvds_pll_clk)
+ clk_disable_unprepare(crtc->dc->hlcdc->lvds_pll_clk);
+ else
+ clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
}
static enum drm_mode_status
@@ -242,7 +251,11 @@ static void atmel_hlcdc_crtc_atomic_disable(struct drm_crtc *c,
10, 1000))
dev_warn(dev->dev, "Atmel LCDC status register CLKSTS timeout\n");
- clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
+ if (crtc->dc->hlcdc->lvds_pll_clk)
+ clk_disable_unprepare(crtc->dc->hlcdc->lvds_pll_clk);
+ else
+ clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
+
pinctrl_pm_select_sleep_state(dev->dev);
pm_runtime_allow(dev->dev);
@@ -255,15 +268,38 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
{
struct drm_device *dev = c->dev;
struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c);
+ struct drm_display_mode *adj = &c->state->adjusted_mode;
struct regmap *regmap = crtc->dc->hlcdc->regmap;
unsigned int status;
+ int ret;
pm_runtime_get_sync(dev->dev);
pm_runtime_forbid(dev->dev);
pinctrl_pm_select_default_state(dev->dev);
- clk_prepare_enable(crtc->dc->hlcdc->sys_clk);
+
+ if (crtc->dc->hlcdc->lvds_pll_clk) {
+ /*
+ * When using LVDS displays, fetch the pixel clock from the panel
+ * and set the LVDS PLL clock rate.
+ * As per the datasheet, LVDS PLL clock is 7x the pixel clock.
+ */
+ ret = clk_set_rate(crtc->dc->hlcdc->lvds_pll_clk,
+ (adj->clock * 7 * 1000));
+ if (ret) {
+ dev_err(dev->dev, "Failed to set LVDS PLL clk rate: %d\n", ret);
+ return;
+ }
+
+ ret = clk_prepare_enable(crtc->dc->hlcdc->lvds_pll_clk);
+ if (ret)
+ return;
+ } else {
+ ret = clk_prepare_enable(crtc->dc->hlcdc->sys_clk);
+ if (ret)
+ return;
+ }
regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_HLCDC_PIXEL_CLK);
if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/3] drm: atmel-hlcdc: add support for LVDS encoder type
2024-11-21 9:23 [PATCH 1/3] drm: atmel-hlcdc: add support for LVDS encoder type Manikandan Muralidharan
2024-11-21 9:23 ` [PATCH 2/3] mfd: atmel-hlcdc: fetch LVDS PLL clock for LVDS display Manikandan Muralidharan
2024-11-21 9:23 ` [PATCH 3/3] drm: atmel-hlcdc: set LVDS PLL clock rate for LVDS Displays Manikandan Muralidharan
@ 2025-01-28 5:03 ` Manikandan.M
2 siblings, 0 replies; 6+ messages in thread
From: Manikandan.M @ 2025-01-28 5:03 UTC (permalink / raw)
To: sam, bbrezillon, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, Nicolas.Ferre, alexandre.belloni, claudiu.beznea, lee,
dri-devel, linux-arm-kernel, linux-kernel
Cc: Dharma.B
Hi,
A gentle reminder to review this series.
On 21/11/24 2:53 pm, Manikandan Muralidharan wrote:
> From: Dharma Balasubiramani <dharma.b@microchip.com>
>
> Add support for encoder type "DRM_MODE_ENCODER_LVDS" with the following
> bus formats:
> - RGB888_1X7X4_SPWG
> - RGB888_1X7X4_JEIDA
> - RGB666_1X7X3_SPWG
> - RGB666_1X18
>
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> [manikandan.m@microchip.com: move modifications inside the
> atmel_xlcdc_connector_output_lvds fn]
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
> .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 38 +++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 0f7ffb3ced20..0e709047369a 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -356,6 +356,42 @@ static int atmel_xlcdc_connector_output_dsi(struct drm_encoder *encoder,
> return supported_fmts;
> }
>
> +static int atmel_xlcdc_connector_output_lvds(struct drm_encoder *encoder,
> + struct drm_display_info *info)
> +{
> + int j;
> + unsigned int supported_fmts = 0;
> +
> + switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
> + case 0:
> + break;
> + case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
> + case MEDIA_BUS_FMT_RGB666_1X18:
> + return ATMEL_HLCDC_RGB666_OUTPUT;
> + case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
> + case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
> + return ATMEL_HLCDC_RGB888_OUTPUT;
> + default:
> + return -EINVAL;
> + }
> +
> + for (j = 0; j < info->num_bus_formats; j++) {
> + switch (info->bus_formats[j]) {
> + case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
> + case MEDIA_BUS_FMT_RGB666_1X18:
> + supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
> + break;
> + case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
> + case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
> + supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
> + break;
> + default:
> + break;
> + }
> + }
> + return supported_fmts;
> +}
> +
> static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
> {
> struct drm_connector *connector = state->connector;
> @@ -374,6 +410,8 @@ static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
> */
> if (encoder->encoder_type == DRM_MODE_ENCODER_DSI)
> return atmel_xlcdc_connector_output_dsi(encoder, info);
> + else if (encoder->encoder_type == DRM_MODE_ENCODER_LVDS)
> + return atmel_xlcdc_connector_output_lvds(encoder, info);
>
> switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
> case 0:
--
Thanks and Regards,
Manikandan M.
^ permalink raw reply [flat|nested] 6+ messages in thread