[PATCH 1/3] drm: rcar-du: lvds: refactor LVDS startup
After the recent corrections to the R-Car gen2/3 LVDS startup code, already
similar enough at their ends rcar_lvds_enable_gen{2|3}() started asking for
a merge and it's becoming actually necessary with the addition of the R-Car
V3M (R8A77970) support -- this gen3 SoC has gen2-like LVDPLLCR layout.
BTW, such a merge saves 64 bytes of the object code with AArch64 gcc 4.8.5.
Signed-off-by: Sergei Shtylyov
---
drivers/gpu/drm/rcar-du/rcar_lvds.c | 137 +++-
1 file changed, 59 insertions(+), 78 deletions(-)
Index: linux/drivers/gpu/drm/rcar-du/rcar_lvds.c
===
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ linux/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -125,98 +125,46 @@ static const struct drm_connector_funcs
* Bridge
*/
-static void rcar_lvds_enable_gen2(struct rcar_lvds *lvds)
+static u32 rcar_lvds_lvdpllcr_gen2(unsigned int freq)
{
- const struct drm_display_mode *mode = >display_mode;
- /*
-* FIXME: We should really retrieve this through the state, but how do
-* we get a state pointer?
-*/
- struct drm_crtc *crtc = lvds->bridge.encoder->crtc;
- unsigned int freq = mode->clock;
- u32 lvdcr0;
- u32 pllcr;
+ u32 lvdpllcr;
- /* PLL clock configuration */
if (freq < 39000)
- pllcr = LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_38M;
+ lvdpllcr = LVDPLLCR_PLLDLYCNT_38M;
else if (freq < 61000)
- pllcr = LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_60M;
+ lvdpllcr = LVDPLLCR_PLLDLYCNT_60M;
else if (freq < 121000)
- pllcr = LVDPLLCR_CEEN | LVDPLLCR_COSEL |
LVDPLLCR_PLLDLYCNT_121M;
+ lvdpllcr = LVDPLLCR_PLLDLYCNT_121M;
else
- pllcr = LVDPLLCR_PLLDLYCNT_150M;
-
- rcar_lvds_write(lvds, LVDPLLCR, pllcr);
-
- /* Turn all the channels on. */
- rcar_lvds_write(lvds, LVDCR1, LVDCR1_CHSTBY(3) | LVDCR1_CHSTBY(2) |
- LVDCR1_CHSTBY(1) | LVDCR1_CHSTBY(0) | LVDCR1_CLKSTBY);
-
- /*
-* Set the LVDS mode, select the input, enable LVDS operation,
-* and turn bias circuitry on.
-*/
- lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_BEN | LVDCR0_LVEN;
- if (drm_crtc_index(crtc) == 2)
- lvdcr0 |= LVDCR0_DUSEL;
- rcar_lvds_write(lvds, LVDCR0, lvdcr0);
-
- /*
-* Turn the PLL on, wait for the startup delay, and turn the output
-* on.
-*/
- lvdcr0 |= LVDCR0_PLLON;
- rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+ return LVDPLLCR_PLLDLYCNT_150M;
- usleep_range(100, 150);
-
- lvdcr0 |= LVDCR0_LVRES;
- rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+ return lvdpllcr | LVDPLLCR_CEEN | LVDPLLCR_COSEL;
}
-static void rcar_lvds_enable_gen3(struct rcar_lvds *lvds)
+static u32 rcar_lvds_lvdpllcr_gen3(unsigned int freq)
{
- const struct drm_display_mode *mode = >display_mode;
- unsigned int freq = mode->clock;
- u32 lvdcr0;
- u32 pllcr;
-
- /* PLL clock configuration */
if (freq < 42000)
- pllcr = LVDPLLCR_PLLDIVCNT_42M;
+ return LVDPLLCR_PLLDIVCNT_42M;
else if (freq < 85000)
- pllcr = LVDPLLCR_PLLDIVCNT_85M;
+ return LVDPLLCR_PLLDIVCNT_85M;
else if (freq < 128000)
- pllcr = LVDPLLCR_PLLDIVCNT_128M;
- else
- pllcr = LVDPLLCR_PLLDIVCNT_148M;
-
- rcar_lvds_write(lvds, LVDPLLCR, pllcr);
-
- /* Turn all the channels on. */
- rcar_lvds_write(lvds, LVDCR1, LVDCR1_CHSTBY(3) | LVDCR1_CHSTBY(2) |
- LVDCR1_CHSTBY(1) | LVDCR1_CHSTBY(0) | LVDCR1_CLKSTBY);
+ return LVDPLLCR_PLLDIVCNT_128M;
- /*
-* Turn the PLL on, set it to LVDS normal mode, wait for the startup
-* delay and turn the output on.
-*/
- lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_PLLON;
- rcar_lvds_write(lvds, LVDCR0, lvdcr0);
-
- lvdcr0 |= LVDCR0_PWD;
- rcar_lvds_write(lvds, LVDCR0, lvdcr0);
-
- usleep_range(100, 150);
-
- lvdcr0 |= LVDCR0_LVRES;
- rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+ return LVDPLLCR_PLLDIVCNT_148M;
}
static void rcar_lvds_enable(struct drm_bridge *bridge)
{
struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
+ /*
+* FIXME: We should really retrieve this through the state, but how do
+* we get a state pointer?
+*/
+ struct drm_crtc *crtc = lvds->bridge.encoder->crtc;
+ const struct drm_display_mode *mode = >display_mode;
+ unsigned int gen = lvds->info->gen;
+ u32 lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
+ u32 lvdpllcr;
u32 lvdhcr;
int ret;
@@ -244,14 +192,47 @@
Re: [PATCH 1/3] drm: rcar-du: lvds: refactor LVDS startup
Hello!
On 1/12/2018 4:26 AM, Laurent Pinchart wrote:
After the recent correction of the R-Car gen3 LVDCR1 value, already similar
enough at their ends rcar_du_lvdsenc_start_gen{2|3}() started asking for a
merge and it's becoming actually necessary with the addition the R-Car V3M
(R8A77970) support -- this gen3 SoC has gen2-like LVDPLLCR layout.
BTW, such a merge saves 72 bytes of the object code with AArch64 gcc 4.8.5.
Signed-off-by: Sergei Shtylyov
---
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 132 +++---
1 file changed, 54 insertions(+), 78 deletions(-)
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
===
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
@@ -39,98 +39,41 @@ static void rcar_lvds_write(struct rcar_
[...]
static int rcar_du_lvdsenc_start(struct rcar_du_lvdsenc *lvds,
struct rcar_du_crtc *rcrtc)
{
- u32 lvdhcr;
+ u32 lvdhcr, lvdpllcr, lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
Please declare variables on separate lines. Especially having lvdcr0 on a line
of its own will make it clearer.
OK.
+ const struct drm_display_mode *mode = >crtc.mode;
+ unsigned int gen = lvds->dev->info->gen;
+ unsigned int freq = mode->clock;
int ret;
if (lvds->enabled)
@@ -158,14 +101,47 @@ static int rcar_du_lvdsenc_start(struct
else
lvdhcr = LVDCHCR_CHSEL_CH(0, 0) | LVDCHCR_CHSEL_CH(1, 1)
| LVDCHCR_CHSEL_CH(2, 2) | LVDCHCR_CHSEL_CH(3, 3);
-
rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
- /* Perform generation-specific initialization. */
- if (lvds->dev->info->gen < 3)
- rcar_du_lvdsenc_start_gen2(lvds, rcrtc);
+ /* PLL clock configuration */
+ if (gen < 3)
+ lvdpllcr = rcar_lvds_gen2_lvdpllcr(freq);
else
- rcar_du_lvdsenc_start_gen3(lvds, rcrtc);
+ lvdpllcr = rcar_lvds_gen3_lvdpllcr(freq);
+ rcar_lvds_write(lvds, LVDPLLCR, lvdpllcr);
+
+ if (gen < 3) {
+ /*
+* Select the input, enable LVDS operation, and turn
+* the bias circuitry on.
+*/
+ lvdcr0 |= LVDCR0_BEN | LVDCR0_LVEN;
+ if (rcrtc->index == 2)
+ lvdcr0 |= LVDCR0_DUSEL;
+ rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+ }
+
+ /* Turn all the channels on. */
+ rcar_lvds_write(lvds, LVDCR1,
+ LVDCR1_CHSTBY(3) | LVDCR1_CHSTBY(2) |
+ LVDCR1_CHSTBY(1) | LVDCR1_CHSTBY(0) | LVDCR1_CLKSTBY);
+
+ /* Turn the PLL on. */
+ lvdcr0 |= LVDCR0_PLLON;
+ rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+
+ if (gen > 2) {
+ /* Turn on the LVDS normal mode. */
+ lvdcr0 |= LVDCR0_PWD;
+ rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+ }
+
+ /* Wait for the startup delay. */
+ usleep_range(100, 150);
+
+ /* Turn the output on. */
+ lvdcr0 |= LVDCR0_LVRES;
+ rcar_lvds_write(lvds, LVDCR0, lvdcr0);
lvds->enabled = true;
How will this work with the D3 and E3 SoCs ? They seem to have a few
differences, will code still be reusable ?
Looking at "37.3.7 Setting Procedure" it will require some adjustments --
in the same vein as in the V3M case. The most prominent D3/E3 specific feature
is the totally different LVDPLLCR layout, and it's not currently supported...
MBR, Sergei
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: rcar-du: lvds: refactor LVDS startup
Hi Sergei,
Thank you for the patch.
On Thursday, 11 January 2018 18:54:33 EET Sergei Shtylyov wrote:
> After the recent correction of the R-Car gen3 LVDCR1 value, already similar
> enough at their ends rcar_du_lvdsenc_start_gen{2|3}() started asking for a
> merge and it's becoming actually necessary with the addition the R-Car V3M
> (R8A77970) support -- this gen3 SoC has gen2-like LVDPLLCR layout.
>
> BTW, such a merge saves 72 bytes of the object code with AArch64 gcc 4.8.5.
>
> Signed-off-by: Sergei Shtylyov
>
> ---
> drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 132 +++---
> 1 file changed, 54 insertions(+), 78 deletions(-)
>
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
> ===
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
> @@ -39,98 +39,41 @@ static void rcar_lvds_write(struct rcar_
> iowrite32(data, lvds->mmio + reg);
> }
>
> -static void rcar_du_lvdsenc_start_gen2(struct rcar_du_lvdsenc *lvds,
> -struct rcar_du_crtc *rcrtc)
> +static u32 rcar_lvds_gen2_lvdpllcr(unsigned int freq)
> {
> - const struct drm_display_mode *mode = >crtc.mode;
> - unsigned int freq = mode->clock;
> - u32 lvdcr0;
> - u32 pllcr;
> + u32 lvdpllcr;
>
> - /* PLL clock configuration */
> if (freq < 39000)
> - pllcr = LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_38M;
> + lvdpllcr = LVDPLLCR_PLLDLYCNT_38M;
> else if (freq < 61000)
> - pllcr = LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_60M;
> + lvdpllcr = LVDPLLCR_PLLDLYCNT_60M;
> else if (freq < 121000)
> - pllcr = LVDPLLCR_CEEN | LVDPLLCR_COSEL |
> LVDPLLCR_PLLDLYCNT_121M;
> + lvdpllcr = LVDPLLCR_PLLDLYCNT_121M;
> else
> - pllcr = LVDPLLCR_PLLDLYCNT_150M;
> -
> - rcar_lvds_write(lvds, LVDPLLCR, pllcr);
> -
> - /*
> - * Select the input, hardcode mode 0, enable LVDS operation and turn
> - * bias circuitry on.
> - */
> - lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_BEN | LVDCR0_LVEN;
> - if (rcrtc->index == 2)
> - lvdcr0 |= LVDCR0_DUSEL;
> - rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> + return LVDPLLCR_PLLDLYCNT_150M;
>
> - /* Turn all the channels on. */
> - rcar_lvds_write(lvds, LVDCR1,
> - LVDCR1_CHSTBY(3) | LVDCR1_CHSTBY(2) |
> - LVDCR1_CHSTBY(1) | LVDCR1_CHSTBY(0) | LVDCR1_CLKSTBY);
> -
> - /*
> - * Turn the PLL on, wait for the startup delay, and turn the output
> - * on.
> - */
> - lvdcr0 |= LVDCR0_PLLON;
> - rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> -
> - usleep_range(100, 150);
> -
> - lvdcr0 |= LVDCR0_LVRES;
> - rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> + return lvdpllcr | LVDPLLCR_CEEN | LVDPLLCR_COSEL;
> }
>
> -static void rcar_du_lvdsenc_start_gen3(struct rcar_du_lvdsenc *lvds,
> -struct rcar_du_crtc *rcrtc)
> +static u32 rcar_lvds_gen3_lvdpllcr(unsigned int freq)
> {
> - const struct drm_display_mode *mode = >crtc.mode;
> - unsigned int freq = mode->clock;
> - u32 lvdcr0;
> - u32 pllcr;
> -
> - /* PLL clock configuration */
> if (freq < 42000)
> - pllcr = LVDPLLCR_PLLDIVCNT_42M;
> + return LVDPLLCR_PLLDIVCNT_42M;
> else if (freq < 85000)
> - pllcr = LVDPLLCR_PLLDIVCNT_85M;
> + return LVDPLLCR_PLLDIVCNT_85M;
> else if (freq < 128000)
> - pllcr = LVDPLLCR_PLLDIVCNT_128M;
> - else
> - pllcr = LVDPLLCR_PLLDIVCNT_148M;
> -
> - rcar_lvds_write(lvds, LVDPLLCR, pllcr);
> -
> - /* Turn all the channels on. */
> - rcar_lvds_write(lvds, LVDCR1,
> - LVDCR1_CHSTBY(3) | LVDCR1_CHSTBY(2) |
> - LVDCR1_CHSTBY(1) | LVDCR1_CHSTBY(0) | LVDCR1_CLKSTBY);
> -
> - /*
> - * Turn the PLL on, set it to LVDS normal mode, wait for the startup
> - * delay and turn the output on.
> - */
> - lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_PLLON;
> - rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> -
> - lvdcr0 |= LVDCR0_PWD;
> - rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> + return LVDPLLCR_PLLDIVCNT_128M;
>
> - usleep_range(100, 150);
> -
> - lvdcr0 |= LVDCR0_LVRES;
> - rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> + return LVDPLLCR_PLLDIVCNT_148M;
> }
>
> static int rcar_du_lvdsenc_start(struct rcar_du_lvdsenc *lvds,
>struct rcar_du_crtc *rcrtc)
> {
> - u32 lvdhcr;
> + u32 lvdhcr, lvdpllcr, lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
Please declare variables on separate lines. Especially having lvdcr0 on a line
of its own will make it clearer.
> +
[PATCH 1/3] drm: rcar-du: lvds: refactor LVDS startup
After the recent correction of the R-Car gen3 LVDCR1 value, already similar
enough at their ends rcar_du_lvdsenc_start_gen{2|3}() started asking for a
merge and it's becoming actually necessary with the addition the R-Car V3M
(R8A77970) support -- this gen3 SoC has gen2-like LVDPLLCR layout.
BTW, such a merge saves 72 bytes of the object code with AArch64 gcc 4.8.5.
Signed-off-by: Sergei Shtylyov
---
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 132 --
1 file changed, 54 insertions(+), 78 deletions(-)
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
===
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
@@ -39,98 +39,41 @@ static void rcar_lvds_write(struct rcar_
iowrite32(data, lvds->mmio + reg);
}
-static void rcar_du_lvdsenc_start_gen2(struct rcar_du_lvdsenc *lvds,
- struct rcar_du_crtc *rcrtc)
+static u32 rcar_lvds_gen2_lvdpllcr(unsigned int freq)
{
- const struct drm_display_mode *mode = >crtc.mode;
- unsigned int freq = mode->clock;
- u32 lvdcr0;
- u32 pllcr;
+ u32 lvdpllcr;
- /* PLL clock configuration */
if (freq < 39000)
- pllcr = LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_38M;
+ lvdpllcr = LVDPLLCR_PLLDLYCNT_38M;
else if (freq < 61000)
- pllcr = LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_60M;
+ lvdpllcr = LVDPLLCR_PLLDLYCNT_60M;
else if (freq < 121000)
- pllcr = LVDPLLCR_CEEN | LVDPLLCR_COSEL |
LVDPLLCR_PLLDLYCNT_121M;
+ lvdpllcr = LVDPLLCR_PLLDLYCNT_121M;
else
- pllcr = LVDPLLCR_PLLDLYCNT_150M;
-
- rcar_lvds_write(lvds, LVDPLLCR, pllcr);
-
- /*
-* Select the input, hardcode mode 0, enable LVDS operation and turn
-* bias circuitry on.
-*/
- lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_BEN | LVDCR0_LVEN;
- if (rcrtc->index == 2)
- lvdcr0 |= LVDCR0_DUSEL;
- rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+ return LVDPLLCR_PLLDLYCNT_150M;
- /* Turn all the channels on. */
- rcar_lvds_write(lvds, LVDCR1,
- LVDCR1_CHSTBY(3) | LVDCR1_CHSTBY(2) |
- LVDCR1_CHSTBY(1) | LVDCR1_CHSTBY(0) | LVDCR1_CLKSTBY);
-
- /*
-* Turn the PLL on, wait for the startup delay, and turn the output
-* on.
-*/
- lvdcr0 |= LVDCR0_PLLON;
- rcar_lvds_write(lvds, LVDCR0, lvdcr0);
-
- usleep_range(100, 150);
-
- lvdcr0 |= LVDCR0_LVRES;
- rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+ return lvdpllcr | LVDPLLCR_CEEN | LVDPLLCR_COSEL;
}
-static void rcar_du_lvdsenc_start_gen3(struct rcar_du_lvdsenc *lvds,
- struct rcar_du_crtc *rcrtc)
+static u32 rcar_lvds_gen3_lvdpllcr(unsigned int freq)
{
- const struct drm_display_mode *mode = >crtc.mode;
- unsigned int freq = mode->clock;
- u32 lvdcr0;
- u32 pllcr;
-
- /* PLL clock configuration */
if (freq < 42000)
- pllcr = LVDPLLCR_PLLDIVCNT_42M;
+ return LVDPLLCR_PLLDIVCNT_42M;
else if (freq < 85000)
- pllcr = LVDPLLCR_PLLDIVCNT_85M;
+ return LVDPLLCR_PLLDIVCNT_85M;
else if (freq < 128000)
- pllcr = LVDPLLCR_PLLDIVCNT_128M;
- else
- pllcr = LVDPLLCR_PLLDIVCNT_148M;
-
- rcar_lvds_write(lvds, LVDPLLCR, pllcr);
-
- /* Turn all the channels on. */
- rcar_lvds_write(lvds, LVDCR1,
- LVDCR1_CHSTBY(3) | LVDCR1_CHSTBY(2) |
- LVDCR1_CHSTBY(1) | LVDCR1_CHSTBY(0) | LVDCR1_CLKSTBY);
-
- /*
-* Turn the PLL on, set it to LVDS normal mode, wait for the startup
-* delay and turn the output on.
-*/
- lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_PLLON;
- rcar_lvds_write(lvds, LVDCR0, lvdcr0);
-
- lvdcr0 |= LVDCR0_PWD;
- rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+ return LVDPLLCR_PLLDIVCNT_128M;
- usleep_range(100, 150);
-
- lvdcr0 |= LVDCR0_LVRES;
- rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+ return LVDPLLCR_PLLDIVCNT_148M;
}
static int rcar_du_lvdsenc_start(struct rcar_du_lvdsenc *lvds,
struct rcar_du_crtc *rcrtc)
{
- u32 lvdhcr;
+ u32 lvdhcr, lvdpllcr, lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
+ const struct drm_display_mode *mode = >crtc.mode;
+ unsigned int gen = lvds->dev->info->gen;
+ unsigned int freq = mode->clock;
int ret;
if (lvds->enabled)
@@ -158,14 +101,47 @@ static int rcar_du_lvdsenc_start(struct
else
lvdhcr = LVDCHCR_CHSEL_CH(0,
