| Message ID | 20170929082306.16193-5-wens@csie.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 29, 2017 at 08:22:56AM +0000, Chen-Yu Tsai wrote: > On systems with 2 TCONs such as the A31, it is possible to demux the > output of the TCONs to one encoder. > > Add support for this for the A31. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > --- > drivers/gpu/drm/sun4i/sun4i_tcon.c | 38 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c > index 7bf51abaee97..c949309d4285 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > @@ -112,6 +112,21 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, bool enable) > } > EXPORT_SYMBOL(sun4i_tcon_enable_vblank); > > +static struct sun4i_tcon *sun4i_get_first_tcon(struct drm_device *drm) Would that make sense to make it a bit more generic, and pass the id to look for as an argument?
On Fri, Sep 29, 2017 at 6:20 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Fri, Sep 29, 2017 at 08:22:56AM +0000, Chen-Yu Tsai wrote: >> On systems with 2 TCONs such as the A31, it is possible to demux the >> output of the TCONs to one encoder. >> >> Add support for this for the A31. >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> --- >> drivers/gpu/drm/sun4i/sun4i_tcon.c | 38 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 38 insertions(+) >> >> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c >> index 7bf51abaee97..c949309d4285 100644 >> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c >> @@ -112,6 +112,21 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, bool enable) >> } >> EXPORT_SYMBOL(sun4i_tcon_enable_vblank); >> >> +static struct sun4i_tcon *sun4i_get_first_tcon(struct drm_device *drm) > > Would that make sense to make it a bit more generic, and pass the id > to look for as an argument? The reason to look for TCON0 explicitly is to access the muxing registers, which are only available in TCON0. Other than that, there's nothing else shared between the two TCONs. So there's no particular reason to look for TCON1 explicitly. ChenYu
Hi Chen-Yu, On Fri, Sep 29, 2017 at 8:22 PM, Chen-Yu Tsai <wens@csie.org> wrote: > On Fri, Sep 29, 2017 at 6:20 PM, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: >> On Fri, Sep 29, 2017 at 08:22:56AM +0000, Chen-Yu Tsai wrote: >>> On systems with 2 TCONs such as the A31, it is possible to demux the >>> output of the TCONs to one encoder. >>> >>> Add support for this for the A31. >>> >>> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >>> --- >>> drivers/gpu/drm/sun4i/sun4i_tcon.c | 38 ++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 38 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c >>> index 7bf51abaee97..c949309d4285 100644 >>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c >>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c >>> @@ -112,6 +112,21 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, bool enable) >>> } >>> EXPORT_SYMBOL(sun4i_tcon_enable_vblank); >>> >>> +static struct sun4i_tcon *sun4i_get_first_tcon(struct drm_device *drm) >> >> Would that make sense to make it a bit more generic, and pass the id >> to look for as an argument? > > The reason to look for TCON0 explicitly is to access the muxing registers, which > are only available in TCON0. Other than that, there's nothing else > shared between > the two TCONs. So there's no particular reason to look for TCON1 explicitly. In that case: in the bizarre case where we're trying to use this mux type and there is no TCON0, shouldn't we fail? (Also, the code doesn't make sense if we have some TCON1 and TCON2 in that order as it'll return TCON2) Thanks,
On Sat, Sep 30, 2017 at 1:35 PM, Julian Calaby <julian.calaby@gmail.com> wrote: > Hi Chen-Yu, > > On Fri, Sep 29, 2017 at 8:22 PM, Chen-Yu Tsai <wens@csie.org> wrote: >> On Fri, Sep 29, 2017 at 6:20 PM, Maxime Ripard >> <maxime.ripard@free-electrons.com> wrote: >>> On Fri, Sep 29, 2017 at 08:22:56AM +0000, Chen-Yu Tsai wrote: >>>> On systems with 2 TCONs such as the A31, it is possible to demux the >>>> output of the TCONs to one encoder. >>>> >>>> Add support for this for the A31. >>>> >>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >>>> --- >>>> drivers/gpu/drm/sun4i/sun4i_tcon.c | 38 ++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 38 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c >>>> index 7bf51abaee97..c949309d4285 100644 >>>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c >>>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c >>>> @@ -112,6 +112,21 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, bool enable) >>>> } >>>> EXPORT_SYMBOL(sun4i_tcon_enable_vblank); >>>> >>>> +static struct sun4i_tcon *sun4i_get_first_tcon(struct drm_device *drm) >>> >>> Would that make sense to make it a bit more generic, and pass the id >>> to look for as an argument? >> >> The reason to look for TCON0 explicitly is to access the muxing registers, which >> are only available in TCON0. Other than that, there's nothing else >> shared between >> the two TCONs. So there's no particular reason to look for TCON1 explicitly. > > In that case: in the bizarre case where we're trying to use this mux > type and there is no TCON0, shouldn't we fail? It gives out a big warning, indicating something is wrong. If TCON0 is not found it is most likely your device tree is broken. There's nothing more the driver can do. Are you suggesting to return NULL in this case, and also do error handling in the callers? > (Also, the code doesn't make sense if we have some TCON1 and TCON2 in > that order as it'll return TCON2) I'm guessing you want it to return NULL. ChenYu
Hi Chen-Yu, On Sat, Sep 30, 2017 at 3:58 PM, Chen-Yu Tsai <wens@csie.org> wrote: > On Sat, Sep 30, 2017 at 1:35 PM, Julian Calaby <julian.calaby@gmail.com> wrote: >> Hi Chen-Yu, >> >> On Fri, Sep 29, 2017 at 8:22 PM, Chen-Yu Tsai <wens@csie.org> wrote: >>> On Fri, Sep 29, 2017 at 6:20 PM, Maxime Ripard >>> <maxime.ripard@free-electrons.com> wrote: >>>> On Fri, Sep 29, 2017 at 08:22:56AM +0000, Chen-Yu Tsai wrote: >>>>> On systems with 2 TCONs such as the A31, it is possible to demux the >>>>> output of the TCONs to one encoder. >>>>> >>>>> Add support for this for the A31. >>>>> >>>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >>>>> --- >>>>> drivers/gpu/drm/sun4i/sun4i_tcon.c | 38 ++++++++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 38 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c >>>>> index 7bf51abaee97..c949309d4285 100644 >>>>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c >>>>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c >>>>> @@ -112,6 +112,21 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, bool enable) >>>>> } >>>>> EXPORT_SYMBOL(sun4i_tcon_enable_vblank); >>>>> >>>>> +static struct sun4i_tcon *sun4i_get_first_tcon(struct drm_device *drm) >>>> >>>> Would that make sense to make it a bit more generic, and pass the id >>>> to look for as an argument? >>> >>> The reason to look for TCON0 explicitly is to access the muxing registers, which >>> are only available in TCON0. Other than that, there's nothing else >>> shared between >>> the two TCONs. So there's no particular reason to look for TCON1 explicitly. >> >> In that case: in the bizarre case where we're trying to use this mux >> type and there is no TCON0, shouldn't we fail? > > It gives out a big warning, indicating something is wrong. If TCON0 is not found > it is most likely your device tree is broken. There's nothing more the > driver can do. > Are you suggesting to return NULL in this case, and also do error > handling in the > callers? You're already returning -EINVAL for other failure cases, so a lack of TCON0 might as well do the same. >> (Also, the code doesn't make sense if we have some TCON1 and TCON2 in >> that order as it'll return TCON2) > > I'm guessing you want it to return NULL. I'm just pointing out the mismatch between getting the "first" TCON and the actual behaviour. Thanks,
On Sat, Sep 30, 2017 at 2:26 PM, Julian Calaby <julian.calaby@gmail.com> wrote: > Hi Chen-Yu, > > On Sat, Sep 30, 2017 at 3:58 PM, Chen-Yu Tsai <wens@csie.org> wrote: >> On Sat, Sep 30, 2017 at 1:35 PM, Julian Calaby <julian.calaby@gmail.com> wrote: >>> Hi Chen-Yu, >>> >>> On Fri, Sep 29, 2017 at 8:22 PM, Chen-Yu Tsai <wens@csie.org> wrote: >>>> On Fri, Sep 29, 2017 at 6:20 PM, Maxime Ripard >>>> <maxime.ripard@free-electrons.com> wrote: >>>>> On Fri, Sep 29, 2017 at 08:22:56AM +0000, Chen-Yu Tsai wrote: >>>>>> On systems with 2 TCONs such as the A31, it is possible to demux the >>>>>> output of the TCONs to one encoder. >>>>>> >>>>>> Add support for this for the A31. >>>>>> >>>>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >>>>>> --- >>>>>> drivers/gpu/drm/sun4i/sun4i_tcon.c | 38 ++++++++++++++++++++++++++++++++++++++ >>>>>> 1 file changed, 38 insertions(+) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c >>>>>> index 7bf51abaee97..c949309d4285 100644 >>>>>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c >>>>>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c >>>>>> @@ -112,6 +112,21 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, bool enable) >>>>>> } >>>>>> EXPORT_SYMBOL(sun4i_tcon_enable_vblank); >>>>>> >>>>>> +static struct sun4i_tcon *sun4i_get_first_tcon(struct drm_device *drm) >>>>> >>>>> Would that make sense to make it a bit more generic, and pass the id >>>>> to look for as an argument? >>>> >>>> The reason to look for TCON0 explicitly is to access the muxing registers, which >>>> are only available in TCON0. Other than that, there's nothing else >>>> shared between >>>> the two TCONs. So there's no particular reason to look for TCON1 explicitly. >>> >>> In that case: in the bizarre case where we're trying to use this mux >>> type and there is no TCON0, shouldn't we fail? >> >> It gives out a big warning, indicating something is wrong. If TCON0 is not found >> it is most likely your device tree is broken. There's nothing more the >> driver can do. >> Are you suggesting to return NULL in this case, and also do error >> handling in the >> callers? > > You're already returning -EINVAL for other failure cases, so a lack of > TCON0 might as well do the same. > >>> (Also, the code doesn't make sense if we have some TCON1 and TCON2 in >>> that order as it'll return TCON2) >> >> I'm guessing you want it to return NULL. > > I'm just pointing out the mismatch between getting the "first" TCON > and the actual behaviour. Makes sense. I've renamed it to "_get_tcon0" and added more comments on it's behavior. Also made it return NULL when tcon0 is not found. ChenYu
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 7bf51abaee97..c949309d4285 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -112,6 +112,21 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, bool enable) } EXPORT_SYMBOL(sun4i_tcon_enable_vblank); +static struct sun4i_tcon *sun4i_get_first_tcon(struct drm_device *drm) +{ + struct sun4i_drv *drv = drm->dev_private; + struct sun4i_tcon *tcon; + + list_for_each_entry(tcon, &drv->tcon_list, list) + if (tcon->id == 0) + return tcon; + + dev_warn(drm->dev, + "TCON0 not found, display output muxing may not work\n"); + + return tcon; +} + void sun4i_tcon_set_mux(struct sun4i_tcon *tcon, int channel, struct drm_encoder *encoder) { @@ -777,6 +792,28 @@ static int sun5i_a13_tcon_set_mux(struct sun4i_tcon *tcon, return regmap_write(tcon->regs, SUN4I_TCON_MUX_CTRL_REG, val); } +static int sun6i_tcon_set_mux(struct sun4i_tcon *tcon, + struct drm_encoder *encoder) +{ + struct sun4i_tcon *tcon0 = sun4i_get_first_tcon(encoder->dev); + u32 shift; + + switch (encoder->encoder_type) { + case DRM_MODE_ENCODER_TMDS: + /* HDMI */ + shift = 8; + break; + default: + /* TODO A31 has MIPI DSI but A31s does not */ + return -EINVAL; + } + + regmap_update_bits(tcon0->regs, SUN4I_TCON_MUX_CTRL_REG, + 0x3 << shift, tcon->id << shift); + + return 0; +} + static const struct sun4i_tcon_quirks sun5i_a13_quirks = { .has_unknown_mux = true, .has_channel_1 = true, @@ -786,6 +823,7 @@ static const struct sun4i_tcon_quirks sun5i_a13_quirks = { static const struct sun4i_tcon_quirks sun6i_a31_quirks = { .has_channel_1 = true, .needs_de_be_mux = true, + .set_mux = sun6i_tcon_set_mux, }; static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
On systems with 2 TCONs such as the A31, it is possible to demux the output of the TCONs to one encoder. Add support for this for the A31. Signed-off-by: Chen-Yu Tsai <wens@csie.org> --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)