Re: [PATCH/RFC 11/15] drm: rcar-du: lvds: Add support for dual-link mode
Hi Jacopo, On Sat, Mar 09, 2019 at 12:11:32PM +0100, Jacopo Mondi wrote: > On Fri, Mar 08, 2019 at 08:12:39PM +0200, Laurent Pinchart wrote: > > On Fri, Mar 08, 2019 at 06:20:23PM +0100, Jacopo Mondi wrote: > >> On Thu, Mar 07, 2019 at 01:23:41AM +0200, Laurent Pinchart wrote: > >>> In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and > >>> sends odd-numbered pixels to the LVDS1 encoder for transmission on a > >>> separate link. > >>> > >>> To implement support for this mode of operation, determine if the LVDS > >>> connection operates in dual-link mode by querying the next device in the > >>> pipeline, locate the companion encoder, and control it directly through > >>> its bridge operations. > >>> > >>> Signed-off-by: Laurent Pinchart > >>> > >>> --- > >>> drivers/gpu/drm/rcar-du/rcar_lvds.c | 104 > >>> drivers/gpu/drm/rcar-du/rcar_lvds.h | 5 ++ > >>> 2 files changed, 96 insertions(+), 13 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c > >>> b/drivers/gpu/drm/rcar-du/rcar_lvds.c > >>> index 5ac92ee15be0..90d33514bb3e 100644 > >>> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c > >>> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c > >>> @@ -66,6 +66,9 @@ struct rcar_lvds { > >>> > >>> struct drm_display_mode display_mode; > >>> enum rcar_lvds_mode mode; > >>> + > >>> + struct drm_bridge *companion; > >>> + bool dual_link; > >>> }; > >>> > >>> #define bridge_to_rcar_lvds(bridge) \ > >>> @@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge > >>> *bridge) > >>> { > >>> struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); > >>> const struct drm_display_mode *mode = &lvds->display_mode; > >>> - /* > >>> - * FIXME: We should really retrieve the CRTC through the state, but how > >>> - * do we get a state pointer? > >>> - */ > >>> - struct drm_crtc *crtc = lvds->bridge.encoder->crtc; > >>> u32 lvdhcr; > >>> u32 lvdcr0; > >>> int ret; > >>> @@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge > >>> *bridge) > >>> if (ret < 0) > >>> return; > >>> > >>> + /* Enable the companion LVDS encoder in dual-link mode. */ > >>> + if (lvds->dual_link && lvds->companion) > >>> + lvds->companion->funcs->enable(lvds->companion); > >>> + > >>> /* > >>>* Hardcode the channels and control signals routing for now. > >>>* > >>> @@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge > >>> *bridge) > >>> rcar_lvds_write(lvds, LVDCHCR, lvdhcr); > >>> > >>> if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) { > >>> - /* Disable dual-link mode. */ > >>> - rcar_lvds_write(lvds, LVDSTRIPE, 0); > >>> + /* > >>> + * Configure vertical stripe based on the mode of operation of > >>> + * the connected device. > >>> + */ > >>> + rcar_lvds_write(lvds, LVDSTRIPE, > >>> + lvds->dual_link ? LVDSTRIPE_ST_ON : 0); > >>> } > >>> > >>> - /* PLL clock configuration. */ > >>> - lvds->info->pll_setup(lvds, mode->clock * 1000); > >>> + /* > >>> + * PLL clock configuration on all instances but the companion in > >>> + * dual-link mode. > >>> + */ > >>> + if (!lvds->dual_link || lvds->companion) > >>> + lvds->info->pll_setup(lvds, mode->clock * 1000); > >>> > >>> /* Set the LVDS mode and select the input. */ > >>> lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT; > >>> - if (drm_crtc_index(crtc) == 2) > >>> - lvdcr0 |= LVDCR0_DUSEL; > >>> + > >>> + 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) > >>> + lvdcr0 |= LVDCR0_DUSEL; > >>> + } > >>> + > >>> rcar_lvds_write(lvds, LVDCR0, lvdcr0); > >>> > >>> /* Turn all the channels on. */ > >>> @@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge > >>> *bridge) > >>> rcar_lvds_write(lvds, LVDCR1, 0); > >>> rcar_lvds_write(lvds, LVDPLLCR, 0); > >>> > >>> + /* Disable the companion LVDS encoder in dual-link mode. */ > >>> + if (lvds->dual_link && lvds->companion) > >>> + lvds->companion->funcs->disable(lvds->companion); > >>> + > >>> clk_disable_unprepare(lvds->clocks.mod); > >>> } > >>> > >>> @@ -628,10 +650,54 @@ static const struct drm_bridge_funcs > >>> rcar_lvds_bridge_ops = { > >>> .mode_set = rcar_lvds_mode_set, > >>> }; > >>> > >>> +bool rcar_lvds_dual_link(struct drm_bridge *bridge) > >>> +{ > >>> + struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); > >>> + > >>> + return lvds->dual_link; > >>> +} > >>> +EXPORT_SYMBOL_GPL(rcar_lvds_dual_link); > >>> + > >>> /* > >>> - > >>> * Probe & Remove > >>> */ > >>> > >>> +static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds) > >>> +{ >
Re: [PATCH/RFC 11/15] drm: rcar-du: lvds: Add support for dual-link mode
Hi Laurent, On Fri, Mar 08, 2019 at 08:12:39PM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > On Fri, Mar 08, 2019 at 06:20:23PM +0100, Jacopo Mondi wrote: > > On Thu, Mar 07, 2019 at 01:23:41AM +0200, Laurent Pinchart wrote: > > > In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and > > > sends odd-numbered pixels to the LVDS1 encoder for transmission on a > > > separate link. > > > > > > To implement support for this mode of operation, determine if the LVDS > > > connection operates in dual-link mode by querying the next device in the > > > pipeline, locate the companion encoder, and control it directly through > > > its bridge operations. > > > > > > Signed-off-by: Laurent Pinchart > > > > > > --- > > > drivers/gpu/drm/rcar-du/rcar_lvds.c | 104 > > > drivers/gpu/drm/rcar-du/rcar_lvds.h | 5 ++ > > > 2 files changed, 96 insertions(+), 13 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c > > > b/drivers/gpu/drm/rcar-du/rcar_lvds.c > > > index 5ac92ee15be0..90d33514bb3e 100644 > > > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c > > > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c > > > @@ -66,6 +66,9 @@ struct rcar_lvds { > > > > > > struct drm_display_mode display_mode; > > > enum rcar_lvds_mode mode; > > > + > > > + struct drm_bridge *companion; > > > + bool dual_link; > > > }; > > > > > > #define bridge_to_rcar_lvds(bridge) \ > > > @@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge > > > *bridge) > > > { > > > struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); > > > const struct drm_display_mode *mode = &lvds->display_mode; > > > - /* > > > - * FIXME: We should really retrieve the CRTC through the state, but how > > > - * do we get a state pointer? > > > - */ > > > - struct drm_crtc *crtc = lvds->bridge.encoder->crtc; > > > u32 lvdhcr; > > > u32 lvdcr0; > > > int ret; > > > @@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge > > > *bridge) > > > if (ret < 0) > > > return; > > > > > > + /* Enable the companion LVDS encoder in dual-link mode. */ > > > + if (lvds->dual_link && lvds->companion) > > > + lvds->companion->funcs->enable(lvds->companion); > > > + > > > /* > > >* Hardcode the channels and control signals routing for now. > > >* > > > @@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge > > > *bridge) > > > rcar_lvds_write(lvds, LVDCHCR, lvdhcr); > > > > > > if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) { > > > - /* Disable dual-link mode. */ > > > - rcar_lvds_write(lvds, LVDSTRIPE, 0); > > > + /* > > > + * Configure vertical stripe based on the mode of operation of > > > + * the connected device. > > > + */ > > > + rcar_lvds_write(lvds, LVDSTRIPE, > > > + lvds->dual_link ? LVDSTRIPE_ST_ON : 0); > > > } > > > > > > - /* PLL clock configuration. */ > > > - lvds->info->pll_setup(lvds, mode->clock * 1000); > > > + /* > > > + * PLL clock configuration on all instances but the companion in > > > + * dual-link mode. > > > + */ > > > + if (!lvds->dual_link || lvds->companion) > > > + lvds->info->pll_setup(lvds, mode->clock * 1000); > > > > > > /* Set the LVDS mode and select the input. */ > > > lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT; > > > - if (drm_crtc_index(crtc) == 2) > > > - lvdcr0 |= LVDCR0_DUSEL; > > > + > > > + 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) > > > + lvdcr0 |= LVDCR0_DUSEL; > > > + } > > > + > > > rcar_lvds_write(lvds, LVDCR0, lvdcr0); > > > > > > /* Turn all the channels on. */ > > > @@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge > > > *bridge) > > > rcar_lvds_write(lvds, LVDCR1, 0); > > > rcar_lvds_write(lvds, LVDPLLCR, 0); > > > > > > + /* Disable the companion LVDS encoder in dual-link mode. */ > > > + if (lvds->dual_link && lvds->companion) > > > + lvds->companion->funcs->disable(lvds->companion); > > > + > > > clk_disable_unprepare(lvds->clocks.mod); > > > } > > > > > > @@ -628,10 +650,54 @@ static const struct drm_bridge_funcs > > > rcar_lvds_bridge_ops = { > > > .mode_set = rcar_lvds_mode_set, > > > }; > > > > > > +bool rcar_lvds_dual_link(struct drm_bridge *bridge) > > > +{ > > > + struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); > > > + > > > + return lvds->dual_link; > > > +} > > > +EXPORT_SYMBOL_GPL(rcar_lvds_dual_link); > > > + > > > /* > > > - > > > * Probe & Remove > > > */ > > > > > > +static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds) > > > +{ > > > + const struct of_device_id *match; > > > + str
Re: [PATCH/RFC 11/15] drm: rcar-du: lvds: Add support for dual-link mode
Hi Jacopo, On Fri, Mar 08, 2019 at 06:20:23PM +0100, Jacopo Mondi wrote: > On Thu, Mar 07, 2019 at 01:23:41AM +0200, Laurent Pinchart wrote: > > In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and > > sends odd-numbered pixels to the LVDS1 encoder for transmission on a > > separate link. > > > > To implement support for this mode of operation, determine if the LVDS > > connection operates in dual-link mode by querying the next device in the > > pipeline, locate the companion encoder, and control it directly through > > its bridge operations. > > > > Signed-off-by: Laurent Pinchart > > --- > > drivers/gpu/drm/rcar-du/rcar_lvds.c | 104 > > drivers/gpu/drm/rcar-du/rcar_lvds.h | 5 ++ > > 2 files changed, 96 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c > > b/drivers/gpu/drm/rcar-du/rcar_lvds.c > > index 5ac92ee15be0..90d33514bb3e 100644 > > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c > > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c > > @@ -66,6 +66,9 @@ struct rcar_lvds { > > > > struct drm_display_mode display_mode; > > enum rcar_lvds_mode mode; > > + > > + struct drm_bridge *companion; > > + bool dual_link; > > }; > > > > #define bridge_to_rcar_lvds(bridge) \ > > @@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) > > { > > struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); > > const struct drm_display_mode *mode = &lvds->display_mode; > > - /* > > -* FIXME: We should really retrieve the CRTC through the state, but how > > -* do we get a state pointer? > > -*/ > > - struct drm_crtc *crtc = lvds->bridge.encoder->crtc; > > u32 lvdhcr; > > u32 lvdcr0; > > int ret; > > @@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) > > if (ret < 0) > > return; > > > > + /* Enable the companion LVDS encoder in dual-link mode. */ > > + if (lvds->dual_link && lvds->companion) > > + lvds->companion->funcs->enable(lvds->companion); > > + > > /* > > * Hardcode the channels and control signals routing for now. > > * > > @@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge > > *bridge) > > rcar_lvds_write(lvds, LVDCHCR, lvdhcr); > > > > if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) { > > - /* Disable dual-link mode. */ > > - rcar_lvds_write(lvds, LVDSTRIPE, 0); > > + /* > > +* Configure vertical stripe based on the mode of operation of > > +* the connected device. > > +*/ > > + rcar_lvds_write(lvds, LVDSTRIPE, > > + lvds->dual_link ? LVDSTRIPE_ST_ON : 0); > > } > > > > - /* PLL clock configuration. */ > > - lvds->info->pll_setup(lvds, mode->clock * 1000); > > + /* > > +* PLL clock configuration on all instances but the companion in > > +* dual-link mode. > > +*/ > > + if (!lvds->dual_link || lvds->companion) > > + lvds->info->pll_setup(lvds, mode->clock * 1000); > > > > /* Set the LVDS mode and select the input. */ > > lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT; > > - if (drm_crtc_index(crtc) == 2) > > - lvdcr0 |= LVDCR0_DUSEL; > > + > > + 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) > > + lvdcr0 |= LVDCR0_DUSEL; > > + } > > + > > rcar_lvds_write(lvds, LVDCR0, lvdcr0); > > > > /* Turn all the channels on. */ > > @@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge > > *bridge) > > rcar_lvds_write(lvds, LVDCR1, 0); > > rcar_lvds_write(lvds, LVDPLLCR, 0); > > > > + /* Disable the companion LVDS encoder in dual-link mode. */ > > + if (lvds->dual_link && lvds->companion) > > + lvds->companion->funcs->disable(lvds->companion); > > + > > clk_disable_unprepare(lvds->clocks.mod); > > } > > > > @@ -628,10 +650,54 @@ static const struct drm_bridge_funcs > > rcar_lvds_bridge_ops = { > > .mode_set = rcar_lvds_mode_set, > > }; > > > > +bool rcar_lvds_dual_link(struct drm_bridge *bridge) > > +{ > > + struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); > > + > > + return lvds->dual_link; > > +} > > +EXPORT_SYMBOL_GPL(rcar_lvds_dual_link); > > + > > /* > > - > > * Probe & Remove > > */ > > > > +static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds) > > +{ > > + const struct of_device_id *match; > > + struct device_node *companion; > > + struct device *dev = lvds->dev; > > + int ret = 0; > > + > > + /* Locate the companion LVDS encoder for dual-link operation, if any. */ > > + companion = of_parse_phandle(dev->of_node, "ren
Re: [PATCH/RFC 11/15] drm: rcar-du: lvds: Add support for dual-link mode
Hi Laurent, On Thu, Mar 07, 2019 at 01:23:41AM +0200, Laurent Pinchart wrote: > In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and > sends odd-numbered pixels to the LVDS1 encoder for transmission on a > separate link. > > To implement support for this mode of operation, determine if the LVDS > connection operates in dual-link mode by querying the next device in the > pipeline, locate the companion encoder, and control it directly through > its bridge operations. > > Signed-off-by: Laurent Pinchart > --- > drivers/gpu/drm/rcar-du/rcar_lvds.c | 104 > drivers/gpu/drm/rcar-du/rcar_lvds.h | 5 ++ > 2 files changed, 96 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c > b/drivers/gpu/drm/rcar-du/rcar_lvds.c > index 5ac92ee15be0..90d33514bb3e 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c > @@ -66,6 +66,9 @@ struct rcar_lvds { > > struct drm_display_mode display_mode; > enum rcar_lvds_mode mode; > + > + struct drm_bridge *companion; > + bool dual_link; > }; > > #define bridge_to_rcar_lvds(bridge) \ > @@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) > { > struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); > const struct drm_display_mode *mode = &lvds->display_mode; > - /* > - * FIXME: We should really retrieve the CRTC through the state, but how > - * do we get a state pointer? > - */ > - struct drm_crtc *crtc = lvds->bridge.encoder->crtc; > u32 lvdhcr; > u32 lvdcr0; > int ret; > @@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) > if (ret < 0) > return; > > + /* Enable the companion LVDS encoder in dual-link mode. */ > + if (lvds->dual_link && lvds->companion) > + lvds->companion->funcs->enable(lvds->companion); > + > /* >* Hardcode the channels and control signals routing for now. >* > @@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) > rcar_lvds_write(lvds, LVDCHCR, lvdhcr); > > if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) { > - /* Disable dual-link mode. */ > - rcar_lvds_write(lvds, LVDSTRIPE, 0); > + /* > + * Configure vertical stripe based on the mode of operation of > + * the connected device. > + */ > + rcar_lvds_write(lvds, LVDSTRIPE, > + lvds->dual_link ? LVDSTRIPE_ST_ON : 0); > } > > - /* PLL clock configuration. */ > - lvds->info->pll_setup(lvds, mode->clock * 1000); > + /* > + * PLL clock configuration on all instances but the companion in > + * dual-link mode. > + */ > + if (!lvds->dual_link || lvds->companion) > + lvds->info->pll_setup(lvds, mode->clock * 1000); > > /* Set the LVDS mode and select the input. */ > lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT; > - if (drm_crtc_index(crtc) == 2) > - lvdcr0 |= LVDCR0_DUSEL; > + > + 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) > + lvdcr0 |= LVDCR0_DUSEL; > + } > + > rcar_lvds_write(lvds, LVDCR0, lvdcr0); > > /* Turn all the channels on. */ > @@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge *bridge) > rcar_lvds_write(lvds, LVDCR1, 0); > rcar_lvds_write(lvds, LVDPLLCR, 0); > > + /* Disable the companion LVDS encoder in dual-link mode. */ > + if (lvds->dual_link && lvds->companion) > + lvds->companion->funcs->disable(lvds->companion); > + > clk_disable_unprepare(lvds->clocks.mod); > } > > @@ -628,10 +650,54 @@ static const struct drm_bridge_funcs > rcar_lvds_bridge_ops = { > .mode_set = rcar_lvds_mode_set, > }; > > +bool rcar_lvds_dual_link(struct drm_bridge *bridge) > +{ > + struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); > + > + return lvds->dual_link; > +} > +EXPORT_SYMBOL_GPL(rcar_lvds_dual_link); > + > /* > - > * Probe & Remove > */ > > +static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds) > +{ > + const struct of_device_id *match; > + struct device_node *companion; > + struct device *dev = lvds->dev; > + int ret = 0; > + > + /* Locate the companion LVDS encoder for dual-link operation, if any. */ > + companion = of_parse_phandle(dev->of_node, "renesas,companion", 0); > + if (!companion) > + return -ENODEV; > + > + /* > + * Sanity check: the companion encoder must have the same compatible > + * string. > +