| Message ID | 20180111170103.676775968@cogentembedded.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sergei, Thank you for the patch. On Thursday, 11 January 2018 18:54:35 EET Sergei Shtylyov wrote: > Add support for the R-Car V3M (R8A77970) SoC to the DU driver (this SoC > has only 1 display port). Note that there are some differences with the > other R-Car gen3 SoCs in the LVDS encoder part, e.g. LVDPLLCR has the same > layout as on the R-Car gen2 SoCs... > > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Could you please rebase this series on top of the LVDS rework posted as "[PATCH 00/10] R-Car DU: Convert LVDS code to bridge driver" (https:// www.spinics.net/lists/dri-devel/msg161931.html) ? It should make it easier to implement support for V3M. Please then split the DU and LVDS drivers changes in two patches. > --- > Documentation/devicetree/bindings/display/renesas,du.txt | 1 Please split the DT bindings changes to a separate patch. > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 23 +++++++++++ > drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 > drivers/gpu/drm/rcar-du/rcar_du_group.c | 10 +++--- > drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 20 +++++++---- > 5 files changed, 45 insertions(+), 10 deletions(-) > > Index: linux/Documentation/devicetree/bindings/display/renesas,du.txt > =================================================================== > --- linux.orig/Documentation/devicetree/bindings/display/renesas,du.txt > +++ linux/Documentation/devicetree/bindings/display/renesas,du.txt > @@ -13,6 +13,7 @@ Required Properties: > - "renesas,du-r8a7794" for R8A7794 (R-Car E2) compatible DU > - "renesas,du-r8a7795" for R8A7795 (R-Car H3) compatible DU > - "renesas,du-r8a7796" for R8A7796 (R-Car M3-W) compatible DU > + - "renesas,du-r8a77970" for R8A77970 (R-Car V3M) compatible DU > > - reg: A list of base address and length of each memory resource, one for > each entry in the reg-names property. You also need to update the ports table further down in this file. > Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c > =================================================================== > --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.c > +++ linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c > @@ -258,6 +258,28 @@ static const struct rcar_du_device_info > .dpll_ch = BIT(1), > }; > > +static const struct rcar_du_device_info rcar_du_r8a77970_info = { > + .gen = 3, > + .model = R8A77970, > + .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > + | RCAR_DU_FEATURE_EXT_CTRL_REGS > + | RCAR_DU_FEATURE_VSP1_SOURCE, > + .num_crtcs = 1, > + .routes = { > + /* R8A77970 has one RGB output and one LVDS output. */ > + [RCAR_DU_OUTPUT_DPAD0] = { > + .possible_crtcs = BIT(0), > + .port = 1, > + }, > + [RCAR_DU_OUTPUT_LVDS0] = { > + .possible_crtcs = BIT(0), > + .port = 0, > + }, All the other SoCs have DPAD0 as port 0. Unless there's a specific need for a different implementation with V3M I'd keep the same order. > + }, > + .num_lvds = 1, > + .dpll_ch = BIT(1), This doesn't seem to be correct, there's no DPLL in V3M. > +}; > + > static const struct of_device_id rcar_du_of_table[] = { > { .compatible = "renesas,du-r8a7743", .data = &rzg1_du_r8a7743_info }, > { .compatible = "renesas,du-r8a7745", .data = &rzg1_du_r8a7745_info }, > @@ -269,6 +291,7 @@ static const struct of_device_id rcar_du > { .compatible = "renesas,du-r8a7794", .data = &rcar_du_r8a7794_info }, > { .compatible = "renesas,du-r8a7795", .data = &rcar_du_r8a7795_info }, > { .compatible = "renesas,du-r8a7796", .data = &rcar_du_r8a7796_info }, > + { .compatible = "renesas,du-r8a77970", .data = &rcar_du_r8a77970_info }, > { } > }; > Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h > =================================================================== > --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.h > +++ linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h > @@ -59,6 +59,7 @@ enum rcar_du_model { > R8A7794, > R8A7795, > R8A7796, > + R8A77970, > }; > > /* > Index: linux/drivers/gpu/drm/rcar-du/rcar_du_group.c > =================================================================== > --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_group.c > +++ linux/drivers/gpu/drm/rcar-du/rcar_du_group.c > @@ -133,10 +133,12 @@ static void rcar_du_group_setup(struct r > rcar_du_group_write(rgrp, DORCR, DORCR_PG1D_DS1 | DORCR_DPRS); > > /* Apply planes to CRTCs association. */ > - mutex_lock(&rgrp->lock); > - rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) | > - rgrp->dptsr_planes); > - mutex_unlock(&rgrp->lock); > + if (rcdu->info->num_crtcs > 1) { > + mutex_lock(&rgrp->lock); > + rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) | > + rgrp->dptsr_planes); > + mutex_unlock(&rgrp->lock); > + } Shouldn't you skip writing to the DPTSR register if there's a single DPTSR in the group ? That would then apply to M3-W as well, which doesn't have the DPTSR2 register. I'd split this change to a separate patch. > } > > /* > 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 > @@ -71,8 +71,10 @@ static int rcar_du_lvdsenc_start(struct > struct rcar_du_crtc *rcrtc) > { > u32 lvdhcr, lvdpllcr, lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT; > + const struct rcar_du_device_info *info = lvds->dev->info; > const struct drm_display_mode *mode = &rcrtc->crtc.mode; > - unsigned int gen = lvds->dev->info->gen; > + unsigned int gen = info->gen; > + enum rcar_du_model model = info->model; > unsigned int freq = mode->clock; > int ret; > > @@ -104,7 +106,7 @@ static int rcar_du_lvdsenc_start(struct > rcar_lvds_write(lvds, LVDCHCR, lvdhcr); > > /* PLL clock configuration */ > - if (gen < 3) > + if (gen < 3 || model == R8A77970) > lvdpllcr = rcar_lvds_gen2_lvdpllcr(freq); > else > lvdpllcr = rcar_lvds_gen3_lvdpllcr(freq); > @@ -136,6 +138,12 @@ static int rcar_du_lvdsenc_start(struct > rcar_lvds_write(lvds, LVDCR0, lvdcr0); > } > > + /* Turn on the LVDS PHY on R-Car V3M. */ > + if (model == R8A77970) { > + lvdcr0 |= LVDCR0_LVEN; > + rcar_lvds_write(lvds, LVDCR0, lvdcr0); > + } > + > /* Wait for the startup delay. */ > usleep_range(100, 150); > > @@ -177,14 +185,14 @@ int rcar_du_lvdsenc_enable(struct rcar_d > void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds, > struct drm_display_mode *mode) > { > - struct rcar_du_device *rcdu = lvds->dev; > + const struct rcar_du_device_info *info = lvds->dev->info; > > /* > * The internal LVDS encoder has a restricted clock frequency operating > - * range (30MHz to 150MHz on Gen2, 25.175MHz to 148.5MHz on Gen3). Clamp > - * the clock accordingly. > + * range (30MHz to 150MHz on Gen2 and R-Car V3M, 25.175MHz to 148.5MHz > + * on Gen3). Clamp the clock accordingly. > */ > - if (rcdu->info->gen < 3) > + if (info->gen < 3 || info->model == R8A77970) > mode->clock = clamp(mode->clock, 30000, 150000); According to the datasheet the frequency range for V3M is the same as for the H3 and M3 SoCs. The range seems to have changed starting in datasheet version 0.52. I would fix the range in a separate patch first. If you want I can send patches to fix this issue and the previous one, or you can write them and include them in v2. Let me know what you'd prefer. > else > mode->clock = clamp(mode->clock, 25175, 148500);
On 1/12/2018 4:13 AM, Laurent Pinchart wrote: >> Add support for the R-Car V3M (R8A77970) SoC to the DU driver (this SoC >> has only 1 display port). Note that there are some differences with the >> other R-Car gen3 SoCs in the LVDS encoder part, e.g. LVDPLLCR has the same >> layout as on the R-Car gen2 SoCs... >> >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > Could you please rebase this series on top of the LVDS rework posted as > "[PATCH 00/10] R-Car DU: Convert LVDS code to bridge driver" (https:// > www.spinics.net/lists/dri-devel/msg161931.html) ? It should make it easier to > implement support for V3M. Please then split the DU and LVDS drivers changes > in two patches. > >> --- >> Documentation/devicetree/bindings/display/renesas,du.txt | 1 > > Please split the DT bindings changes to a separate patch. I don't like putting a one-line chnage into a separate bindings patch... >> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 23 +++++++++++ >> drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 >> drivers/gpu/drm/rcar-du/rcar_du_group.c | 10 +++--- >> drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 20 +++++++---- >> 5 files changed, 45 insertions(+), 10 deletions(-) >> >> Index: linux/Documentation/devicetree/bindings/display/renesas,du.txt >> =================================================================== >> --- linux.orig/Documentation/devicetree/bindings/display/renesas,du.txt >> +++ linux/Documentation/devicetree/bindings/display/renesas,du.txt >> @@ -13,6 +13,7 @@ Required Properties: >> - "renesas,du-r8a7794" for R8A7794 (R-Car E2) compatible DU >> - "renesas,du-r8a7795" for R8A7795 (R-Car H3) compatible DU >> - "renesas,du-r8a7796" for R8A7796 (R-Car M3-W) compatible DU >> + - "renesas,du-r8a77970" for R8A77970 (R-Car V3M) compatible DU >> >> - reg: A list of base address and length of each memory resource, one for >> each entry in the reg-names property. > > You also need to update the ports table further down in this file. ... but this one seems to justify dpoing it that way. :-) >> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c >> =================================================================== >> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.c >> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c >> @@ -258,6 +258,28 @@ static const struct rcar_du_device_info >> .dpll_ch = BIT(1), >> }; >> >> +static const struct rcar_du_device_info rcar_du_r8a77970_info = { >> + .gen = 3, >> + .model = R8A77970, >> + .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK >> + | RCAR_DU_FEATURE_EXT_CTRL_REGS >> + | RCAR_DU_FEATURE_VSP1_SOURCE, >> + .num_crtcs = 1, >> + .routes = { >> + /* R8A77970 has one RGB output and one LVDS output. */ >> + [RCAR_DU_OUTPUT_DPAD0] = { >> + .possible_crtcs = BIT(0), >> + .port = 1, >> + }, >> + [RCAR_DU_OUTPUT_LVDS0] = { >> + .possible_crtcs = BIT(0), >> + .port = 0, >> + }, > > All the other SoCs have DPAD0 as port 0. Unless there's a specific need for a > different implementation with V3M I'd keep the same order. I'll look into this.. >> + }, >> + .num_lvds = 1, >> + .dpll_ch = BIT(1), > > This doesn't seem to be correct, there's no DPLL in V3M. Indeed, thanks! [...] >> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_group.c >> =================================================================== >> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_group.c >> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_group.c >> @@ -133,10 +133,12 @@ static void rcar_du_group_setup(struct r >> rcar_du_group_write(rgrp, DORCR, DORCR_PG1D_DS1 | DORCR_DPRS); >> >> /* Apply planes to CRTCs association. */ >> - mutex_lock(&rgrp->lock); >> - rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) | >> - rgrp->dptsr_planes); >> - mutex_unlock(&rgrp->lock); >> + if (rcdu->info->num_crtcs > 1) { >> + mutex_lock(&rgrp->lock); >> + rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) | >> + rgrp->dptsr_planes); >> + mutex_unlock(&rgrp->lock); >> + } > > Shouldn't you skip writing to the DPTSR register if there's a single DPTSR in > the group ? That would then apply to M3-W as well, which doesn't have the > DPTSR2 register. I'd split this change to a separate patch. OK, I guess you know this stuff better -- I didn't know DPTSR2 is used at all... :-) [...] >> 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 [...] >> @@ -177,14 +185,14 @@ int rcar_du_lvdsenc_enable(struct rcar_d >> void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds, >> struct drm_display_mode *mode) >> { >> - struct rcar_du_device *rcdu = lvds->dev; >> + const struct rcar_du_device_info *info = lvds->dev->info; >> >> /* >> * The internal LVDS encoder has a restricted clock frequency operating >> - * range (30MHz to 150MHz on Gen2, 25.175MHz to 148.5MHz on Gen3). Clamp >> - * the clock accordingly. >> + * range (30MHz to 150MHz on Gen2 and R-Car V3M, 25.175MHz to 148.5MHz >> + * on Gen3). Clamp the clock accordingly. >> */ >> - if (rcdu->info->gen < 3) >> + if (info->gen < 3 || info->model == R8A77970) >> mode->clock = clamp(mode->clock, 30000, 150000); > > According to the datasheet the frequency range for V3M is the same as for the > H3 and M3 SoCs. Indeed! I thought it's determined by the LVDPLLCR layout but it's not... > The range seems to have changed starting in datasheet version > 0.52. I would fix the range in a separate patch first. Yes. > If you want I can send patches to fix this issue Yes, please. You clearly know about DU more than me. :-) > and the previous one, or you > can write them and include them in v2. Let me know what you'd prefer. > >> else >> mode->clock = clamp(mode->clock, 25175, 148500); The lower bound documented on gen3 is 31 MHz indeed... MBR, Sergei
Hi Sergei, On Friday, 12 January 2018 11:23:00 EET Sergei Shtylyov wrote: > On 1/12/2018 4:13 AM, Laurent Pinchart wrote: > >> Add support for the R-Car V3M (R8A77970) SoC to the DU driver (this SoC > >> has only 1 display port). Note that there are some differences with the > >> other R-Car gen3 SoCs in the LVDS encoder part, e.g. LVDPLLCR has the > >> same layout as on the R-Car gen2 SoCs... > >> > >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > > > Could you please rebase this series on top of the LVDS rework posted as > > "[PATCH 00/10] R-Car DU: Convert LVDS code to bridge driver" (https:// > > www.spinics.net/lists/dri-devel/msg161931.html) ? It should make it easier > > to implement support for V3M. Please then split the DU and LVDS drivers > > changes in two patches. > > > >> --- > >> > >> Documentation/devicetree/bindings/display/renesas,du.txt | 1 > > > > Please split the DT bindings changes to a separate patch. > > I don't like putting a one-line chnage into a separate bindings patch... > >> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 23 +++++++ > >> drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 > >> drivers/gpu/drm/rcar-du/rcar_du_group.c | 10 +++--- > >> drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 20 ++++--- > >> 5 files changed, 45 insertions(+), 10 deletions(-) [snip] > >> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_group.c > >> =================================================================== > >> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_group.c > >> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_group.c > >> @@ -133,10 +133,12 @@ static void rcar_du_group_setup(struct r > >> > >> rcar_du_group_write(rgrp, DORCR, DORCR_PG1D_DS1 | DORCR_DPRS); > >> > >> /* Apply planes to CRTCs association. */ > >> > >> - mutex_lock(&rgrp->lock); > >> - rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) | > >> - rgrp->dptsr_planes); > >> - mutex_unlock(&rgrp->lock); > >> + if (rcdu->info->num_crtcs > 1) { > >> + mutex_lock(&rgrp->lock); > >> + rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) | > >> + rgrp->dptsr_planes); > >> + mutex_unlock(&rgrp->lock); > >> + } > > > > Shouldn't you skip writing to the DPTSR register if there's a single DPTSR > > in the group ? That would then apply to M3-W as well, which doesn't have > > the DPTSR2 register. I'd split this change to a separate patch. > > OK, I guess you know this stuff better -- I didn't know DPTSR2 is used > at all... :-) Should I send a patch for this as well ? > [...] > > >> 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 > > [...] > > >> @@ -177,14 +185,14 @@ int rcar_du_lvdsenc_enable(struct rcar_d > >> void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds, > >> struct drm_display_mode *mode) > >> { > >> - struct rcar_du_device *rcdu = lvds->dev; > >> + const struct rcar_du_device_info *info = lvds->dev->info; > >> > >> /* > >> * The internal LVDS encoder has a restricted clock frequency > >> operating > >> - * range (30MHz to 150MHz on Gen2, 25.175MHz to 148.5MHz on Gen3). > >> Clamp > >> - * the clock accordingly. > >> + * range (30MHz to 150MHz on Gen2 and R-Car V3M, 25.175MHz to 148.5MHz > >> + * on Gen3). Clamp the clock accordingly. > >> */ > >> - if (rcdu->info->gen < 3) > >> + if (info->gen < 3 || info->model == R8A77970) > >> mode->clock = clamp(mode->clock, 30000, 150000); > > > > According to the datasheet the frequency range for V3M is the same as for > > the H3 and M3 SoCs. > > Indeed! I thought it's determined by the LVDPLLCR layout but it's not... > > > The range seems to have changed starting in datasheet version > > 0.52. I would fix the range in a separate patch first. > > Yes. > > > If you want I can send patches to fix this issue > > Yes, please. You clearly know about DU more than me. :-) I've prepared a patch, I'm testing it now and I'll then send it. > > and the previous one, or you can write them and include them in v2. Let me > > know what you'd prefer. > > > >> else > >> mode->clock = clamp(mode->clock, 25175, 148500); > > The lower bound documented on gen3 is 31 MHz indeed... And I just found out that the latest versions of the Gen2 datasheets also document the 31 MHz - 148.5 MHz range. This will simplify the code.
On 01/12/2018 05:30 PM, Laurent Pinchart wrote: >>>> Add support for the R-Car V3M (R8A77970) SoC to the DU driver (this SoC >>>> has only 1 display port). Note that there are some differences with the >>>> other R-Car gen3 SoCs in the LVDS encoder part, e.g. LVDPLLCR has the >>>> same layout as on the R-Car gen2 SoCs... >>>> >>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >>> >>> Could you please rebase this series on top of the LVDS rework posted as >>> "[PATCH 00/10] R-Car DU: Convert LVDS code to bridge driver" (https:// >>> www.spinics.net/lists/dri-devel/msg161931.html) ? It should make it easier >>> to implement support for V3M. Please then split the DU and LVDS drivers >>> changes in two patches. >>> >>>> --- >>>> >>>> Documentation/devicetree/bindings/display/renesas,du.txt | 1 >>> >>> Please split the DT bindings changes to a separate patch. >> >> I don't like putting a one-line chnage into a separate bindings patch... [...] >>>> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_group.c >>>> =================================================================== >>>> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_group.c >>>> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_group.c >>>> @@ -133,10 +133,12 @@ static void rcar_du_group_setup(struct r >>>> >>>> rcar_du_group_write(rgrp, DORCR, DORCR_PG1D_DS1 | DORCR_DPRS); >>>> >>>> /* Apply planes to CRTCs association. */ >>>> >>>> - mutex_lock(&rgrp->lock); >>>> - rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) | >>>> - rgrp->dptsr_planes); >>>> - mutex_unlock(&rgrp->lock); >>>> + if (rcdu->info->num_crtcs > 1) { >>>> + mutex_lock(&rgrp->lock); >>>> + rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) | >>>> + rgrp->dptsr_planes); >>>> + mutex_unlock(&rgrp->lock); >>>> + } >>> >>> Shouldn't you skip writing to the DPTSR register if there's a single DPTSR >>> in the group ? That would then apply to M3-W as well, which doesn't have >>> the DPTSR2 register. I'd split this change to a separate patch. >> >> OK, I guess you know this stuff better -- I didn't know DPTSR2 is used >> at all... :-) > > Should I send a patch for this as well ? I thought that was a "previous" issue you meant? Please fix this too, if it's not too much trouble. :-) >> [...] >> >>>> 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 >> >> [...] >> >>>> @@ -177,14 +185,14 @@ int rcar_du_lvdsenc_enable(struct rcar_d >>>> void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds, >>>> struct drm_display_mode *mode) >>>> { >>>> - struct rcar_du_device *rcdu = lvds->dev; >>>> + const struct rcar_du_device_info *info = lvds->dev->info; >>>> >>>> /* >>>> * The internal LVDS encoder has a restricted clock frequency >>>> operating >>>> - * range (30MHz to 150MHz on Gen2, 25.175MHz to 148.5MHz on Gen3). >>>> Clamp >>>> - * the clock accordingly. >>>> + * range (30MHz to 150MHz on Gen2 and R-Car V3M, 25.175MHz to 148.5MHz >>>> + * on Gen3). Clamp the clock accordingly. >>>> */ >>>> - if (rcdu->info->gen < 3) >>>> + if (info->gen < 3 || info->model == R8A77970) >>>> mode->clock = clamp(mode->clock, 30000, 150000); >>> >>> According to the datasheet the frequency range for V3M is the same as for >>> the H3 and M3 SoCs. >> >> Indeed! I thought it's determined by the LVDPLLCR layout but it's not... >> >>> The range seems to have changed starting in datasheet version >>> 0.52. I would fix the range in a separate patch first. >> >> Yes. >> >>> If you want I can send patches to fix this issue >> >> Yes, please. You clearly know about DU more than me. :-) > > I've prepared a patch, I'm testing it now and I'll then send it. > >>> and the previous one, or you can write them and include them in v2. Let me Here you're talking about the previous issue -- I figured it was about DPTSR... >>> know what you'd prefer. >>> >>>> else >>>> mode->clock = clamp(mode->clock, 25175, 148500); >> >> The lower bound documented on gen3 is 31 MHz indeed... > > And I just found out that the latest versions of the Gen2 datasheets also > document the 31 MHz - 148.5 MHz range. This will simplify the code. Indeed! Thanks for looking. :-) MBR, Sergei
Index: linux/Documentation/devicetree/bindings/display/renesas,du.txt =================================================================== --- linux.orig/Documentation/devicetree/bindings/display/renesas,du.txt +++ linux/Documentation/devicetree/bindings/display/renesas,du.txt @@ -13,6 +13,7 @@ Required Properties: - "renesas,du-r8a7794" for R8A7794 (R-Car E2) compatible DU - "renesas,du-r8a7795" for R8A7795 (R-Car H3) compatible DU - "renesas,du-r8a7796" for R8A7796 (R-Car M3-W) compatible DU + - "renesas,du-r8a77970" for R8A77970 (R-Car V3M) compatible DU - reg: A list of base address and length of each memory resource, one for each entry in the reg-names property. Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c =================================================================== --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -258,6 +258,28 @@ static const struct rcar_du_device_info .dpll_ch = BIT(1), }; +static const struct rcar_du_device_info rcar_du_r8a77970_info = { + .gen = 3, + .model = R8A77970, + .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK + | RCAR_DU_FEATURE_EXT_CTRL_REGS + | RCAR_DU_FEATURE_VSP1_SOURCE, + .num_crtcs = 1, + .routes = { + /* R8A77970 has one RGB output and one LVDS output. */ + [RCAR_DU_OUTPUT_DPAD0] = { + .possible_crtcs = BIT(0), + .port = 1, + }, + [RCAR_DU_OUTPUT_LVDS0] = { + .possible_crtcs = BIT(0), + .port = 0, + }, + }, + .num_lvds = 1, + .dpll_ch = BIT(1), +}; + static const struct of_device_id rcar_du_of_table[] = { { .compatible = "renesas,du-r8a7743", .data = &rzg1_du_r8a7743_info }, { .compatible = "renesas,du-r8a7745", .data = &rzg1_du_r8a7745_info }, @@ -269,6 +291,7 @@ static const struct of_device_id rcar_du { .compatible = "renesas,du-r8a7794", .data = &rcar_du_r8a7794_info }, { .compatible = "renesas,du-r8a7795", .data = &rcar_du_r8a7795_info }, { .compatible = "renesas,du-r8a7796", .data = &rcar_du_r8a7796_info }, + { .compatible = "renesas,du-r8a77970", .data = &rcar_du_r8a77970_info }, { } }; Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h =================================================================== --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.h +++ linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h @@ -59,6 +59,7 @@ enum rcar_du_model { R8A7794, R8A7795, R8A7796, + R8A77970, }; /* Index: linux/drivers/gpu/drm/rcar-du/rcar_du_group.c =================================================================== --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_group.c +++ linux/drivers/gpu/drm/rcar-du/rcar_du_group.c @@ -133,10 +133,12 @@ static void rcar_du_group_setup(struct r rcar_du_group_write(rgrp, DORCR, DORCR_PG1D_DS1 | DORCR_DPRS); /* Apply planes to CRTCs association. */ - mutex_lock(&rgrp->lock); - rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) | - rgrp->dptsr_planes); - mutex_unlock(&rgrp->lock); + if (rcdu->info->num_crtcs > 1) { + mutex_lock(&rgrp->lock); + rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) | + rgrp->dptsr_planes); + mutex_unlock(&rgrp->lock); + } } /* 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 @@ -71,8 +71,10 @@ static int rcar_du_lvdsenc_start(struct struct rcar_du_crtc *rcrtc) { u32 lvdhcr, lvdpllcr, lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT; + const struct rcar_du_device_info *info = lvds->dev->info; const struct drm_display_mode *mode = &rcrtc->crtc.mode; - unsigned int gen = lvds->dev->info->gen; + unsigned int gen = info->gen; + enum rcar_du_model model = info->model; unsigned int freq = mode->clock; int ret; @@ -104,7 +106,7 @@ static int rcar_du_lvdsenc_start(struct rcar_lvds_write(lvds, LVDCHCR, lvdhcr); /* PLL clock configuration */ - if (gen < 3) + if (gen < 3 || model == R8A77970) lvdpllcr = rcar_lvds_gen2_lvdpllcr(freq); else lvdpllcr = rcar_lvds_gen3_lvdpllcr(freq); @@ -136,6 +138,12 @@ static int rcar_du_lvdsenc_start(struct rcar_lvds_write(lvds, LVDCR0, lvdcr0); } + /* Turn on the LVDS PHY on R-Car V3M. */ + if (model == R8A77970) { + lvdcr0 |= LVDCR0_LVEN; + rcar_lvds_write(lvds, LVDCR0, lvdcr0); + } + /* Wait for the startup delay. */ usleep_range(100, 150); @@ -177,14 +185,14 @@ int rcar_du_lvdsenc_enable(struct rcar_d void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds, struct drm_display_mode *mode) { - struct rcar_du_device *rcdu = lvds->dev; + const struct rcar_du_device_info *info = lvds->dev->info; /* * The internal LVDS encoder has a restricted clock frequency operating - * range (30MHz to 150MHz on Gen2, 25.175MHz to 148.5MHz on Gen3). Clamp - * the clock accordingly. + * range (30MHz to 150MHz on Gen2 and R-Car V3M, 25.175MHz to 148.5MHz + * on Gen3). Clamp the clock accordingly. */ - if (rcdu->info->gen < 3) + if (info->gen < 3 || info->model == R8A77970) mode->clock = clamp(mode->clock, 30000, 150000); else mode->clock = clamp(mode->clock, 25175, 148500);
Add support for the R-Car V3M (R8A77970) SoC to the DU driver (this SoC has only 1 display port). Note that there are some differences with the other R-Car gen3 SoCs in the LVDS encoder part, e.g. LVDPLLCR has the same layout as on the R-Car gen2 SoCs... Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- Documentation/devicetree/bindings/display/renesas,du.txt | 1 drivers/gpu/drm/rcar-du/rcar_du_drv.c | 23 +++++++++++++++ drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 drivers/gpu/drm/rcar-du/rcar_du_group.c | 10 +++--- drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 20 +++++++++---- 5 files changed, 45 insertions(+), 10 deletions(-)