Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On 5/10/21 8:04 PM, Dave Stevenson wrote:
Hi,
[...]
+static void sn65dsi83_enable(struct drm_bridge *bridge)
+{
+ struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
+ unsigned int pval;
+ u16 val;
+ int ret;
+
+ /* Clear reset, disable PLL */
+ regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
+ regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
Sorry, a further thread of discussion coming from the investigations
I've been involved with.
You've powered up in pre_enable, and are sending the I2C writes in enable.
>From the docs for drm_bridge_funcs->enable[1]
* The bridge can assume that the display pipe (i.e. clocks and timing
* signals) feeding it is running when this callback is called. This
* callback must enable the display link feeding the next bridge in the
* chain if there is one.
So video is running when enable is called, and the DSI data lanes may
be HS. (Someone correct me if that is an incorrect reading of the
text).
The SN65DSI84 datasheet table 7-2 Initialization Sequence gives init
seq 8 as being "Change DSI data lanes to HS state and start DSI video
stream", AFTER all the I2C has been completed except reading back
registers and checking for errors.
With video running you don't fulfil the second part of init seq 2 "the
DSI data lanes MUST be driven to LP11 state"
My investigations have been over delaying starting the DSI video
stream until after enable, but reading the descriptive text for enable
I believe the Pi is correct to be sending video at that point.
I guess there is some ambiguity as to whether the clock lane is going
to be in HS mode during pre_enable. On the Pi the PHY and clocks will
be enabled prior to pre_enable to allow for sending DSI commands
during pre_enable, but it may not be true on other platforms.
You have to make sure the clock lane is running and in HS mode when
configuring the DSI83, otherwise the internal DSI83 state machine won't
be able to operate.
Indeed, but my reading of the documentation says that neither
pre_enable nor enable give you the state that you require.
You need a hook in the middle, an option to ask for clock lanes during
pre_enable or no video during enable, or an amendment to the docs over
the state during enable.
Having the data lanes in HS mode does appear to stop the DSI83
accepting the I2C setup commands.
Uhh, that is new. Is that what you observed in your lab ?
I saw the DSI83 behave this way if the clock lane was stopped, but the
data lanes had no impact. Was your clock lane running when the DSI83 was
not accepting i2c commands ? Does your DSI83 source clock from it or
from external Xtal ?
I haven't got into the lab as yet, and I don't have a DSI83 myself.
This is relaying experimentation from others.
They're using the DSI clock lane as the clock source.Yes the clock
lane on the Pi is started before any of the enable bridge calls.
In the vc4 driver[1] it runs through the all pre-enables, configures
register DISP0_CTRL including setting bit DSI_DISP0_ENABLE which
starts it requesting pixels from the pipeline, and then calls all the
enables. With that behaviour it fails to start the DSI83.
If the DSI83 I2C setup code is moved from enable to pre_enable then it
works, or if patch [2] is used to move the setting of the
DSI_DISP0_ENABLE bit to after enable it also works.
The pre_enable option won't work on MX8M and I suspect Exynos5, because
that DSIM PHY only enables the DSI HS clock in its bridge enable
(whether that is OK or not, I cannot tell, I am hoping someone can
clarify that).
Sorry life is all rather up in the air with working from home. I'll go
into the lab and try to confirm that DSI_DISP0_ENABLE does what the
documentation implies it does.
Those who do have hardware now have it working on the Pi, although
with a version of Jagan's driver rather than yours. We're trying to
figure out the diffs with yours.
All right, if you figure it out, I'd be interested to know what it is.
If you have it working reliably on other platforms that you believe
are following the docs during pre_enable and enable, then I'm happy to
drop out of the discussions for now. We can revisit it once we have
determined exactly why it's being fussy on the Pi.
Since you have one working setup and another non-working, and the only
difference is the DSI83 bridge driver, it should be possible to find the
difference easily.
I had a look at the driver from Jagan again, and there is no
configuration in pre_enable either, so the pre_enable is likely not the
reason why it works for you. Maybe the extra mdelays the driver adds all
over the place are the reason ?
Cheers
Dave
[1]
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/vc4/vc4_dsi.c#L1072
[2]
https://github.com/6by9/linux/commit/b939eaffc47cc84ebfea6bf1ab10ae1ec9fa58c2
[...]
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On Mon, 10 May 2021 at 12:16, Marek Vasut wrote:
>
> On 5/10/21 11:58 AM, Dave Stevenson wrote:
> > On Sat, 8 May 2021 at 21:26, Marek Vasut wrote:
> >>
> >> On 5/7/21 2:48 PM, Dave Stevenson wrote:
> >>
> >> [...]
> >>
> +static void sn65dsi83_enable(struct drm_bridge *bridge)
> +{
> + struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
> + unsigned int pval;
> + u16 val;
> + int ret;
> +
> + /* Clear reset, disable PLL */
> + regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
> + regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
> >>>
> >>> Sorry, a further thread of discussion coming from the investigations
> >>> I've been involved with.
> >>>
> >>> You've powered up in pre_enable, and are sending the I2C writes in enable.
> >>>
> >>> >From the docs for drm_bridge_funcs->enable[1]
> >>>
> >>>* The bridge can assume that the display pipe (i.e. clocks and timing
> >>>* signals) feeding it is running when this callback is called. This
> >>>* callback must enable the display link feeding the next bridge in the
> >>>* chain if there is one.
> >>>
> >>> So video is running when enable is called, and the DSI data lanes may
> >>> be HS. (Someone correct me if that is an incorrect reading of the
> >>> text).
> >>>
> >>> The SN65DSI84 datasheet table 7-2 Initialization Sequence gives init
> >>> seq 8 as being "Change DSI data lanes to HS state and start DSI video
> >>> stream", AFTER all the I2C has been completed except reading back
> >>> registers and checking for errors.
> >>> With video running you don't fulfil the second part of init seq 2 "the
> >>> DSI data lanes MUST be driven to LP11 state"
> >>>
> >>> My investigations have been over delaying starting the DSI video
> >>> stream until after enable, but reading the descriptive text for enable
> >>> I believe the Pi is correct to be sending video at that point.
> >>> I guess there is some ambiguity as to whether the clock lane is going
> >>> to be in HS mode during pre_enable. On the Pi the PHY and clocks will
> >>> be enabled prior to pre_enable to allow for sending DSI commands
> >>> during pre_enable, but it may not be true on other platforms.
> >>
> >> You have to make sure the clock lane is running and in HS mode when
> >> configuring the DSI83, otherwise the internal DSI83 state machine won't
> >> be able to operate.
> >
> > Indeed, but my reading of the documentation says that neither
> > pre_enable nor enable give you the state that you require.
> > You need a hook in the middle, an option to ask for clock lanes during
> > pre_enable or no video during enable, or an amendment to the docs over
> > the state during enable.
> >
> > Having the data lanes in HS mode does appear to stop the DSI83
> > accepting the I2C setup commands.
>
> Uhh, that is new. Is that what you observed in your lab ?
>
> I saw the DSI83 behave this way if the clock lane was stopped, but the
> data lanes had no impact. Was your clock lane running when the DSI83 was
> not accepting i2c commands ? Does your DSI83 source clock from it or
> from external Xtal ?
I haven't got into the lab as yet, and I don't have a DSI83 myself.
This is relaying experimentation from others.
They're using the DSI clock lane as the clock source.Yes the clock
lane on the Pi is started before any of the enable bridge calls.
In the vc4 driver[1] it runs through the all pre-enables, configures
register DISP0_CTRL including setting bit DSI_DISP0_ENABLE which
starts it requesting pixels from the pipeline, and then calls all the
enables. With that behaviour it fails to start the DSI83.
If the DSI83 I2C setup code is moved from enable to pre_enable then it
works, or if patch [2] is used to move the setting of the
DSI_DISP0_ENABLE bit to after enable it also works.
Sorry life is all rather up in the air with working from home. I'll go
into the lab and try to confirm that DSI_DISP0_ENABLE does what the
documentation implies it does.
Those who do have hardware now have it working on the Pi, although
with a version of Jagan's driver rather than yours. We're trying to
figure out the diffs with yours.
If you have it working reliably on other platforms that you believe
are following the docs during pre_enable and enable, then I'm happy to
drop out of the discussions for now. We can revisit it once we have
determined exactly why it's being fussy on the Pi.
Cheers
Dave
[1]
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/vc4/vc4_dsi.c#L1072
[2]
https://github.com/6by9/linux/commit/b939eaffc47cc84ebfea6bf1ab10ae1ec9fa58c2
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On 5/10/21 11:58 AM, Dave Stevenson wrote:
On Sat, 8 May 2021 at 21:26, Marek Vasut wrote:
On 5/7/21 2:48 PM, Dave Stevenson wrote:
[...]
+static void sn65dsi83_enable(struct drm_bridge *bridge)
+{
+ struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
+ unsigned int pval;
+ u16 val;
+ int ret;
+
+ /* Clear reset, disable PLL */
+ regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
+ regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
Sorry, a further thread of discussion coming from the investigations
I've been involved with.
You've powered up in pre_enable, and are sending the I2C writes in enable.
>From the docs for drm_bridge_funcs->enable[1]
* The bridge can assume that the display pipe (i.e. clocks and timing
* signals) feeding it is running when this callback is called. This
* callback must enable the display link feeding the next bridge in the
* chain if there is one.
So video is running when enable is called, and the DSI data lanes may
be HS. (Someone correct me if that is an incorrect reading of the
text).
The SN65DSI84 datasheet table 7-2 Initialization Sequence gives init
seq 8 as being "Change DSI data lanes to HS state and start DSI video
stream", AFTER all the I2C has been completed except reading back
registers and checking for errors.
With video running you don't fulfil the second part of init seq 2 "the
DSI data lanes MUST be driven to LP11 state"
My investigations have been over delaying starting the DSI video
stream until after enable, but reading the descriptive text for enable
I believe the Pi is correct to be sending video at that point.
I guess there is some ambiguity as to whether the clock lane is going
to be in HS mode during pre_enable. On the Pi the PHY and clocks will
be enabled prior to pre_enable to allow for sending DSI commands
during pre_enable, but it may not be true on other platforms.
You have to make sure the clock lane is running and in HS mode when
configuring the DSI83, otherwise the internal DSI83 state machine won't
be able to operate.
Indeed, but my reading of the documentation says that neither
pre_enable nor enable give you the state that you require.
You need a hook in the middle, an option to ask for clock lanes during
pre_enable or no video during enable, or an amendment to the docs over
the state during enable.
Having the data lanes in HS mode does appear to stop the DSI83
accepting the I2C setup commands.
Uhh, that is new. Is that what you observed in your lab ?
I saw the DSI83 behave this way if the clock lane was stopped, but the
data lanes had no impact. Was your clock lane running when the DSI83 was
not accepting i2c commands ? Does your DSI83 source clock from it or
from external Xtal ?
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On Sat, 8 May 2021 at 21:26, Marek Vasut wrote:
>
> On 5/7/21 2:48 PM, Dave Stevenson wrote:
>
> [...]
>
> >> +static void sn65dsi83_enable(struct drm_bridge *bridge)
> >> +{
> >> + struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
> >> + unsigned int pval;
> >> + u16 val;
> >> + int ret;
> >> +
> >> + /* Clear reset, disable PLL */
> >> + regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
> >> + regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
> >
> > Sorry, a further thread of discussion coming from the investigations
> > I've been involved with.
> >
> > You've powered up in pre_enable, and are sending the I2C writes in enable.
> >
> >>From the docs for drm_bridge_funcs->enable[1]
> >
> > * The bridge can assume that the display pipe (i.e. clocks and timing
> > * signals) feeding it is running when this callback is called. This
> > * callback must enable the display link feeding the next bridge in the
> > * chain if there is one.
> >
> > So video is running when enable is called, and the DSI data lanes may
> > be HS. (Someone correct me if that is an incorrect reading of the
> > text).
> >
> > The SN65DSI84 datasheet table 7-2 Initialization Sequence gives init
> > seq 8 as being "Change DSI data lanes to HS state and start DSI video
> > stream", AFTER all the I2C has been completed except reading back
> > registers and checking for errors.
> > With video running you don't fulfil the second part of init seq 2 "the
> > DSI data lanes MUST be driven to LP11 state"
> >
> > My investigations have been over delaying starting the DSI video
> > stream until after enable, but reading the descriptive text for enable
> > I believe the Pi is correct to be sending video at that point.
> > I guess there is some ambiguity as to whether the clock lane is going
> > to be in HS mode during pre_enable. On the Pi the PHY and clocks will
> > be enabled prior to pre_enable to allow for sending DSI commands
> > during pre_enable, but it may not be true on other platforms.
>
> You have to make sure the clock lane is running and in HS mode when
> configuring the DSI83, otherwise the internal DSI83 state machine won't
> be able to operate.
Indeed, but my reading of the documentation says that neither
pre_enable nor enable give you the state that you require.
You need a hook in the middle, an option to ask for clock lanes during
pre_enable or no video during enable, or an amendment to the docs over
the state during enable.
Having the data lanes in HS mode does appear to stop the DSI83
accepting the I2C setup commands.
Dave
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On 5/7/21 2:48 PM, Dave Stevenson wrote:
[...]
+static void sn65dsi83_enable(struct drm_bridge *bridge)
+{
+ struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
+ unsigned int pval;
+ u16 val;
+ int ret;
+
+ /* Clear reset, disable PLL */
+ regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
+ regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
Sorry, a further thread of discussion coming from the investigations
I've been involved with.
You've powered up in pre_enable, and are sending the I2C writes in enable.
From the docs for drm_bridge_funcs->enable[1]
* The bridge can assume that the display pipe (i.e. clocks and timing
* signals) feeding it is running when this callback is called. This
* callback must enable the display link feeding the next bridge in the
* chain if there is one.
So video is running when enable is called, and the DSI data lanes may
be HS. (Someone correct me if that is an incorrect reading of the
text).
The SN65DSI84 datasheet table 7-2 Initialization Sequence gives init
seq 8 as being "Change DSI data lanes to HS state and start DSI video
stream", AFTER all the I2C has been completed except reading back
registers and checking for errors.
With video running you don't fulfil the second part of init seq 2 "the
DSI data lanes MUST be driven to LP11 state"
My investigations have been over delaying starting the DSI video
stream until after enable, but reading the descriptive text for enable
I believe the Pi is correct to be sending video at that point.
I guess there is some ambiguity as to whether the clock lane is going
to be in HS mode during pre_enable. On the Pi the PHY and clocks will
be enabled prior to pre_enable to allow for sending DSI commands
during pre_enable, but it may not be true on other platforms.
You have to make sure the clock lane is running and in HS mode when
configuring the DSI83, otherwise the internal DSI83 state machine won't
be able to operate.
[...]
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On 5/7/21 11:17 AM, Dave Stevenson wrote: On Thu, 6 May 2021 at 21:51, Marek Vasut wrote: On 5/6/21 6:03 PM, Frieder Schrempf wrote: On 06.05.21 17:46, Marek Vasut wrote: On 5/6/21 5:38 PM, Frieder Schrempf wrote: [...] Works on i.MX8MM with SN65DSI84 and a single link LVDS display (1024x600) and from my perspective everything else also looks good. Thanks for your work! I have two remarks: 1. In my test I couldn't get it to work with four DSI lanes enabled (only with two) but I'm quite sure that the DSIM driver is to blame as everything on the bridge level looks good (also setting the DSI EQ register didn't help as you suggested, Marek). I suspect there is indeed something with the DSIM going on, I'll keep you posted if I find something out. 2. When I set MEDIA_BUS_FMT_RGB888_1X7X4_SPWG in the panel driver I get distorted colors. I need to use MEDIA_BUS_FMT_RGB888_1X24 to make it work, but this is not valid for LVDS. Again I don't think this driver is to blame as I can't see where it does anything wrong, but my experience here is very limited so I still want to mention it. Hmm, in that conversion supposed to happen in this bridge driver or should MXSFB handle the SPWG pixel format ? Or should the DSIM bridge do something about that ? As far as I understand it the conversion is already done by the DSI84 without any extra configuration necessary. The only thing that needs to be done is selecting the LVDS output format via CHx_24BPP_MODE and CHx_24BPP_FORMAT1 which the driver currently hardcodes to 24bpp aka MEDIA_BUS_FMT_RGB888_1X7X4_SPWG. I think the DSI input format is always 24bpp aka MEDIA_BUS_FMT_RGB888_1X24. The DSI is MEDIA_BUS_FMT_RGB888_1X24, yes. So maybe this bridge driver has to somehow deal with MEDIA_BUS_FMT_RGB888_1X7X4_SPWG ? Except I haven't seen such a thing implemented in other bridge drivers, so input would be welcome on this. I'm claiming no knowledge of whether this is the correct approach or not, but Toshiba TC358775 is also a DSI to LVDS bridge which appears to handle both formats. https://elixir.free-electrons.com/linux/latest/source/drivers/gpu/drm/bridge/tc358775.c#L457 That's what quick git grep points you to, yes. Except that driver does not patch the bus pixel format for the DSI in any way, it just passes whatever bus pixel format it got from the panel/output bridge through. You need something like drm_display_info_set_bus_formats() called somewhere. So I wonder where the format actually is evaluated. Could it be that it is passed down to the LCDIF and changes its output format which causes the data passed by DSIM to the DSI84 to already be in the SPWG format? If that's the case we maybe need a way to specify MEDIA_BUS_FMT_RGB888_1X24 as input bus format for the DSI84 so it doesn't pass on the panel's format? Only a wild guess, no idea if it really works like that. I _think_ the bridge must somehow handle the MEDIA_BUS_FMT_RGB888_1X7X4_SPWG <-> MEDIA_BUS_FMT_RGB888_1X24 conversion. I've not looked at where the interchange happens, but as you're setting the DSI format in struct mipi_dsi_device to MIPI_DSI_FMT_RGB888 doesn't that provide the configuration side to the DSI transmitter? Otherwise presumably it needs to support the atomic_get_input_bus_fmts and/or atomic_get_output_bus_fmts functions in drm_bridge_funcs. That doesn't work either, but see above, I think you need drm_display_info_set_bus_formats() .
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On Wed, 5 May 2021 at 11:03, Marek Vasut wrote:
>
> Add driver for TI SN65DSI83 Single-link DSI to Single-link LVDS bridge
> and TI SN65DSI84 Single-link DSI to Dual-link or 2x Single-link LVDS
> bridge. TI SN65DSI85 is unsupported due to lack of hardware to test on,
> but easy to add.
>
> The driver operates the chip via I2C bus. Currently the LVDS clock are
> always derived from DSI clock lane, which is the usual mode of operation.
> Support for clock from external oscillator is not implemented, but it is
> easy to add if ever needed. Only RGB888 pixel format is implemented, the
> LVDS666 is not supported, but could be added if needed.
>
> Signed-off-by: Marek Vasut
> Cc: Douglas Anderson
> Cc: Jagan Teki
> Cc: Laurent Pinchart
> Cc: Linus Walleij
> Cc: Loic Poulain
> Cc: Philippe Schenker
> Cc: Sam Ravnborg
> Cc: Stephen Boyd
> Cc: Valentin Raevsky
> To: dri-devel@lists.freedesktop.org
> ---
> +
> +static void sn65dsi83_pre_enable(struct drm_bridge *bridge)
> +{
> + struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
> +
> + /*
> +* Reset the chip, pull EN line low for t_reset=10ms,
> +* then high for t_en=1ms.
> +*/
> + regcache_mark_dirty(ctx->regmap);
> + gpiod_set_value(ctx->enable_gpio, 0);
> + usleep_range(1, 11000);
> + gpiod_set_value(ctx->enable_gpio, 1);
> + usleep_range(1000, 1100);
> +}
> +
> +
> +static void sn65dsi83_enable(struct drm_bridge *bridge)
> +{
> + struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
> + unsigned int pval;
> + u16 val;
> + int ret;
> +
> + /* Clear reset, disable PLL */
> + regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
> + regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
Sorry, a further thread of discussion coming from the investigations
I've been involved with.
You've powered up in pre_enable, and are sending the I2C writes in enable.
>From the docs for drm_bridge_funcs->enable[1]
* The bridge can assume that the display pipe (i.e. clocks and timing
* signals) feeding it is running when this callback is called. This
* callback must enable the display link feeding the next bridge in the
* chain if there is one.
So video is running when enable is called, and the DSI data lanes may
be HS. (Someone correct me if that is an incorrect reading of the
text).
The SN65DSI84 datasheet table 7-2 Initialization Sequence gives init
seq 8 as being "Change DSI data lanes to HS state and start DSI video
stream", AFTER all the I2C has been completed except reading back
registers and checking for errors.
With video running you don't fulfil the second part of init seq 2 "the
DSI data lanes MUST be driven to LP11 state"
My investigations have been over delaying starting the DSI video
stream until after enable, but reading the descriptive text for enable
I believe the Pi is correct to be sending video at that point.
I guess there is some ambiguity as to whether the clock lane is going
to be in HS mode during pre_enable. On the Pi the PHY and clocks will
be enabled prior to pre_enable to allow for sending DSI commands
during pre_enable, but it may not be true on other platforms.
Dave
[1] https://elixir.bootlin.com/linux/latest/source/include/drm/drm_bridge.h#L256
> +
> + /* Reference clock derived from DSI link clock. */
> + regmap_write(ctx->regmap, REG_RC_LVDS_PLL,
> + REG_RC_LVDS_PLL_LVDS_CLK_RANGE(sn65dsi83_get_lvds_range(ctx))
> |
> + REG_RC_LVDS_PLL_HS_CLK_SRC_DPHY);
> + regmap_write(ctx->regmap, REG_DSI_CLK,
> + REG_DSI_CLK_CHA_DSI_CLK_RANGE(sn65dsi83_get_dsi_range(ctx)));
> + regmap_write(ctx->regmap, REG_RC_DSI_CLK,
> + REG_RC_DSI_CLK_DSI_CLK_DIVIDER(sn65dsi83_get_dsi_div(ctx)));
> +
> + /* Set number of DSI lanes and LVDS link config. */
> + regmap_write(ctx->regmap, REG_DSI_LANE,
> + REG_DSI_LANE_LVDS_LINK_CFG_DUAL |
> + REG_DSI_LANE_CHA_DSI_LANES(~(ctx->dsi_lanes - 1)) |
> + /* CHB is DSI85-only, set to default on DSI83/DSI84 */
> + REG_DSI_LANE_CHB_DSI_LANES(3));
> + /* No equalization. */
> + regmap_write(ctx->regmap, REG_DSI_EQ, 0x00);
> +
> + /* RGB888 is the only format supported so far. */
> + val = (ctx->mode.flags & DRM_MODE_FLAG_NHSYNC ?
> + REG_LVDS_FMT_HS_NEG_POLARITY : 0) |
> + (ctx->mode.flags & DRM_MODE_FLAG_NVSYNC ?
> + REG_LVDS_FMT_VS_NEG_POLARITY : 0) |
> + REG_LVDS_FMT_CHA_24BPP_MODE;
> + if (ctx->lvds_dual_link)
> + val |= REG_LVDS_FMT_CHB_24BPP_MODE;
> + else
> + val |= REG_LVDS_FMT_LVDS_LINK_CFG;
> +
> + regmap_write(ctx->regmap, REG_LVDS_FMT, val);
> + regmap_write(ctx->regmap, REG_LVDS_VCOM, 0x05);
> + regmap_write(ctx->regmap, REG_LVDS_LANE,
> + (ctx->lvds_dual_link_even_odd_swap ?
> +
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On Thu, 6 May 2021 at 21:51, Marek Vasut wrote: > > On 5/6/21 6:03 PM, Frieder Schrempf wrote: > > On 06.05.21 17:46, Marek Vasut wrote: > >> On 5/6/21 5:38 PM, Frieder Schrempf wrote: > >> [...] > >>> Works on i.MX8MM with SN65DSI84 and a single link LVDS display (1024x600) > >>> and from my perspective everything else also looks good. Thanks for your > >>> work! > >>> > >>> I have two remarks: > >>> > >>> 1. In my test I couldn't get it to work with four DSI lanes enabled (only > >>> with two) but I'm quite sure that the DSIM driver is to blame as > >>> everything on the bridge level looks good (also setting the DSI EQ > >>> register didn't help as you suggested, Marek). > >> > >> I suspect there is indeed something with the DSIM going on, I'll keep you > >> posted if I find something out. > >> > >>> 2. When I set MEDIA_BUS_FMT_RGB888_1X7X4_SPWG in the panel driver I get > >>> distorted colors. I need to use MEDIA_BUS_FMT_RGB888_1X24 to make it > >>> work, but this is not valid for LVDS. Again I don't think this driver is > >>> to blame as I can't see where it does anything wrong, but my experience > >>> here is very limited so I still want to mention it. > >> > >> Hmm, in that conversion supposed to happen in this bridge driver or should > >> MXSFB handle the SPWG pixel format ? Or should the DSIM bridge do > >> something about that ? > > > > As far as I understand it the conversion is already done by the DSI84 > > without any extra configuration necessary. The only thing that needs to be > > done is selecting the LVDS output format via CHx_24BPP_MODE and > > CHx_24BPP_FORMAT1 which the driver currently hardcodes to 24bpp aka > > MEDIA_BUS_FMT_RGB888_1X7X4_SPWG. I think the DSI input format is always > > 24bpp aka MEDIA_BUS_FMT_RGB888_1X24. > > The DSI is MEDIA_BUS_FMT_RGB888_1X24, yes. > > So maybe this bridge driver has to somehow deal with > MEDIA_BUS_FMT_RGB888_1X7X4_SPWG ? Except I haven't seen such a thing > implemented in other bridge drivers, so input would be welcome on this. I'm claiming no knowledge of whether this is the correct approach or not, but Toshiba TC358775 is also a DSI to LVDS bridge which appears to handle both formats. https://elixir.free-electrons.com/linux/latest/source/drivers/gpu/drm/bridge/tc358775.c#L457 > > So I wonder where the format actually is evaluated. Could it be that it is > > passed down to the LCDIF and changes its output format which causes the > > data passed by DSIM to the DSI84 to already be in the SPWG format? If > > that's the case we maybe need a way to specify MEDIA_BUS_FMT_RGB888_1X24 as > > input bus format for the DSI84 so it doesn't pass on the panel's format? > > Only a wild guess, no idea if it really works like that. > > I _think_ the bridge must somehow handle the > MEDIA_BUS_FMT_RGB888_1X7X4_SPWG <-> MEDIA_BUS_FMT_RGB888_1X24 conversion. I've not looked at where the interchange happens, but as you're setting the DSI format in struct mipi_dsi_device to MIPI_DSI_FMT_RGB888 doesn't that provide the configuration side to the DSI transmitter? Otherwise presumably it needs to support the atomic_get_input_bus_fmts and/or atomic_get_output_bus_fmts functions in drm_bridge_funcs. Dave
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On Thu, 6 May 2021 at 21:49, Marek Vasut wrote:
>
> On 5/6/21 7:03 PM, Dave Stevenson wrote:
> > On Thu, 6 May 2021 at 13:48, Marek Vasut wrote:
> >>
> >> On 5/6/21 11:45 AM, Dave Stevenson wrote:
> >>> Hi Marek
> >>
> >> Hi,
> >>
> >>> I'm taking an interest as there are a number of Raspberry Pi users
> >>> trying to get this chip up and running (not there quite yet).
> >>> A couple of fairly minor comments
> >>
> >> Is there any readily available display unit / expansion board with this
> >> chip ?
> >
> > Not that I'm aware of. It's a forum thread[1] where two different
> > users are trying to bring up the chip, each with their own boards. One
> > has just reported they have got it working with Jagan's patch set but
> > with a load of hacks to both bridge and DSI drivers.
> > If Laurent has a board then that may be a useful further test route.
> >
> > I'm not 100% convinced that the Pi is doing exactly the right thing
> > with regard LP-11 state during initialisation, but hope to get into
> > the lab to check either tomorrow or early next week.
>
> Good
>
> > [1] https://www.raspberrypi.org/forums/viewtopic.php?f=44=305690
> >
> >> [...]
> >>
> +#define REG_DSI_LANE 0x10
> +#define REG_DSI_LANE_LVDS_LINK_CFG_DUAL BIT(5) /* dual or 2x
> single */
> >>>
> >>> The bit name here seems a little odd.
> >>> Bits 5&6 are the DSI channel mode on SN65DSI85, not the LVDS link
> >>> config (which appears to be reg 0x18 bit 4)
> >>> DSI_CHANNEL_MODE
> >>> 00 – Dual-channel DSI receiver
> >>> 01 – Single channel DSI receiver (default)
> >>> 10 – Two single channel DSI receivers
> >>> 11 – Reserved
> >>> SN65DSI83 and 84 require it to be set to 01. You have that end result,
> >>> but using an odd register name that only documents one of the 2 bits.
> >>>
> >>> Is it worth documenting bit 7 as being the '85 Dual DSI link
> >>> LEFT_RIGHT_PIXELS flag at the same time? The chip isn't supported in
> >>> dual DSI mode at present, but defining all the registers up front
> >>> seems reasonable.
> >>
> >> Yes, let's fix those.
> >>
> >> [...]
> >>
> + /*
> +* Reset the chip, pull EN line low for t_reset=10ms,
> +* then high for t_en=1ms.
> +*/
> + regcache_mark_dirty(ctx->regmap);
> + gpiod_set_value(ctx->enable_gpio, 0);
> + usleep_range(1, 11000);
> + gpiod_set_value(ctx->enable_gpio, 1);
> + usleep_range(1000, 1100);
> +}
> >>>
> >>> Whilst section 6.6 of the SN65DSI84 datasheet does list t_en as 1ms,
> >>> the initialization sequence listed in table 7.2 states
> >>> Init seq 3 - Set EN pin to Low
> >>> Wait 10 ms
> >>> Init seq 4 - Tie EN pin to High
> >>> Wait 10 ms
> >>>
> >>> with the note that these are "Minimum recommended delay. It is fine to
> >>> exceed these."
> >>>
> >>> Have you had alternate guidance from TI over that delay?
> >>
> >> No, I trusted the timing diagrams in section 6.6 of the datasheet. I
> >> would like to believe those are correct, while the init sequence listing
> >> might be a copy-paste error.
> >
> > It's a tough one to call as to which is going to be correct. I just
> > thought I'd flag it.
> >
> >> [...]
> >>
> +static void sn65dsi83_enable(struct drm_bridge *bridge)
> +{
> + struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
> + unsigned int pval;
> + u16 val;
> + int ret;
> +
> + /* Clear reset, disable PLL */
> + regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
> + regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
> +
> + /* Reference clock derived from DSI link clock. */
> + regmap_write(ctx->regmap, REG_RC_LVDS_PLL,
> +
> REG_RC_LVDS_PLL_LVDS_CLK_RANGE(sn65dsi83_get_lvds_range(ctx)) |
> + REG_RC_LVDS_PLL_HS_CLK_SRC_DPHY);
> >>>
> >>> (Checkpatch whinge for "Alignment should match open parenthesis" on
> >>> several lines through this function)
> >>
> >> Do you have any extra checkpatch settings somewhere ?
> >> I get this on current next:
> >>
> >> linux-2.6$ ./scripts/checkpatch.pl -f drivers/gpu/drm/bridge/ti-sn65dsi83.c
> >> total: 0 errors, 0 warnings, 625 lines checked
> >
> > Sorry, yes "./scripts/checkpatch.pl --strict --codespell". Too much
> > working in the linux-media realms where --strict is required :-/
>
> So I can add a variable , assign it the value of
> sn65dsi83_get_lvds_range(ctx) and then do
> REG_RC_LVDS_PLL_LVDS_CLK_RANGE(val), but that doesn't really improve the
> readability at all, it just adds extra indirection.
Unless drm is sticking hard to the older limit, "bdc48fa11e46
checkpatch/coding-style: deprecate 80-column warning" allows you up to
100 chars.
Just going with the natural indentation is therefore fine and makes
checkpatch happy even in strict mode.
Dave
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On 5/6/21 6:03 PM, Frieder Schrempf wrote: On 06.05.21 17:46, Marek Vasut wrote: On 5/6/21 5:38 PM, Frieder Schrempf wrote: [...] Works on i.MX8MM with SN65DSI84 and a single link LVDS display (1024x600) and from my perspective everything else also looks good. Thanks for your work! I have two remarks: 1. In my test I couldn't get it to work with four DSI lanes enabled (only with two) but I'm quite sure that the DSIM driver is to blame as everything on the bridge level looks good (also setting the DSI EQ register didn't help as you suggested, Marek). I suspect there is indeed something with the DSIM going on, I'll keep you posted if I find something out. 2. When I set MEDIA_BUS_FMT_RGB888_1X7X4_SPWG in the panel driver I get distorted colors. I need to use MEDIA_BUS_FMT_RGB888_1X24 to make it work, but this is not valid for LVDS. Again I don't think this driver is to blame as I can't see where it does anything wrong, but my experience here is very limited so I still want to mention it. Hmm, in that conversion supposed to happen in this bridge driver or should MXSFB handle the SPWG pixel format ? Or should the DSIM bridge do something about that ? As far as I understand it the conversion is already done by the DSI84 without any extra configuration necessary. The only thing that needs to be done is selecting the LVDS output format via CHx_24BPP_MODE and CHx_24BPP_FORMAT1 which the driver currently hardcodes to 24bpp aka MEDIA_BUS_FMT_RGB888_1X7X4_SPWG. I think the DSI input format is always 24bpp aka MEDIA_BUS_FMT_RGB888_1X24. The DSI is MEDIA_BUS_FMT_RGB888_1X24, yes. So maybe this bridge driver has to somehow deal with MEDIA_BUS_FMT_RGB888_1X7X4_SPWG ? Except I haven't seen such a thing implemented in other bridge drivers, so input would be welcome on this. So I wonder where the format actually is evaluated. Could it be that it is passed down to the LCDIF and changes its output format which causes the data passed by DSIM to the DSI84 to already be in the SPWG format? If that's the case we maybe need a way to specify MEDIA_BUS_FMT_RGB888_1X24 as input bus format for the DSI84 so it doesn't pass on the panel's format? Only a wild guess, no idea if it really works like that. I _think_ the bridge must somehow handle the MEDIA_BUS_FMT_RGB888_1X7X4_SPWG <-> MEDIA_BUS_FMT_RGB888_1X24 conversion.
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On 5/6/21 7:03 PM, Dave Stevenson wrote:
On Thu, 6 May 2021 at 13:48, Marek Vasut wrote:
On 5/6/21 11:45 AM, Dave Stevenson wrote:
Hi Marek
Hi,
I'm taking an interest as there are a number of Raspberry Pi users
trying to get this chip up and running (not there quite yet).
A couple of fairly minor comments
Is there any readily available display unit / expansion board with this
chip ?
Not that I'm aware of. It's a forum thread[1] where two different
users are trying to bring up the chip, each with their own boards. One
has just reported they have got it working with Jagan's patch set but
with a load of hacks to both bridge and DSI drivers.
If Laurent has a board then that may be a useful further test route.
I'm not 100% convinced that the Pi is doing exactly the right thing
with regard LP-11 state during initialisation, but hope to get into
the lab to check either tomorrow or early next week.
Good
[1] https://www.raspberrypi.org/forums/viewtopic.php?f=44=305690
[...]
+#define REG_DSI_LANE 0x10
+#define REG_DSI_LANE_LVDS_LINK_CFG_DUAL BIT(5) /* dual or 2x single */
The bit name here seems a little odd.
Bits 5&6 are the DSI channel mode on SN65DSI85, not the LVDS link
config (which appears to be reg 0x18 bit 4)
DSI_CHANNEL_MODE
00 – Dual-channel DSI receiver
01 – Single channel DSI receiver (default)
10 – Two single channel DSI receivers
11 – Reserved
SN65DSI83 and 84 require it to be set to 01. You have that end result,
but using an odd register name that only documents one of the 2 bits.
Is it worth documenting bit 7 as being the '85 Dual DSI link
LEFT_RIGHT_PIXELS flag at the same time? The chip isn't supported in
dual DSI mode at present, but defining all the registers up front
seems reasonable.
Yes, let's fix those.
[...]
+ /*
+* Reset the chip, pull EN line low for t_reset=10ms,
+* then high for t_en=1ms.
+*/
+ regcache_mark_dirty(ctx->regmap);
+ gpiod_set_value(ctx->enable_gpio, 0);
+ usleep_range(1, 11000);
+ gpiod_set_value(ctx->enable_gpio, 1);
+ usleep_range(1000, 1100);
+}
Whilst section 6.6 of the SN65DSI84 datasheet does list t_en as 1ms,
the initialization sequence listed in table 7.2 states
Init seq 3 - Set EN pin to Low
Wait 10 ms
Init seq 4 - Tie EN pin to High
Wait 10 ms
with the note that these are "Minimum recommended delay. It is fine to
exceed these."
Have you had alternate guidance from TI over that delay?
No, I trusted the timing diagrams in section 6.6 of the datasheet. I
would like to believe those are correct, while the init sequence listing
might be a copy-paste error.
It's a tough one to call as to which is going to be correct. I just
thought I'd flag it.
[...]
+static void sn65dsi83_enable(struct drm_bridge *bridge)
+{
+ struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
+ unsigned int pval;
+ u16 val;
+ int ret;
+
+ /* Clear reset, disable PLL */
+ regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
+ regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
+
+ /* Reference clock derived from DSI link clock. */
+ regmap_write(ctx->regmap, REG_RC_LVDS_PLL,
+ REG_RC_LVDS_PLL_LVDS_CLK_RANGE(sn65dsi83_get_lvds_range(ctx)) |
+ REG_RC_LVDS_PLL_HS_CLK_SRC_DPHY);
(Checkpatch whinge for "Alignment should match open parenthesis" on
several lines through this function)
Do you have any extra checkpatch settings somewhere ?
I get this on current next:
linux-2.6$ ./scripts/checkpatch.pl -f drivers/gpu/drm/bridge/ti-sn65dsi83.c
total: 0 errors, 0 warnings, 625 lines checked
Sorry, yes "./scripts/checkpatch.pl --strict --codespell". Too much
working in the linux-media realms where --strict is required :-/
So I can add a variable , assign it the value of
sn65dsi83_get_lvds_range(ctx) and then do
REG_RC_LVDS_PLL_LVDS_CLK_RANGE(val), but that doesn't really improve the
readability at all, it just adds extra indirection.
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On Thu, 6 May 2021 at 13:48, Marek Vasut wrote:
>
> On 5/6/21 11:45 AM, Dave Stevenson wrote:
> > Hi Marek
>
> Hi,
>
> > I'm taking an interest as there are a number of Raspberry Pi users
> > trying to get this chip up and running (not there quite yet).
> > A couple of fairly minor comments
>
> Is there any readily available display unit / expansion board with this
> chip ?
Not that I'm aware of. It's a forum thread[1] where two different
users are trying to bring up the chip, each with their own boards. One
has just reported they have got it working with Jagan's patch set but
with a load of hacks to both bridge and DSI drivers.
If Laurent has a board then that may be a useful further test route.
I'm not 100% convinced that the Pi is doing exactly the right thing
with regard LP-11 state during initialisation, but hope to get into
the lab to check either tomorrow or early next week.
[1] https://www.raspberrypi.org/forums/viewtopic.php?f=44=305690
> [...]
>
> >> +#define REG_DSI_LANE 0x10
> >> +#define REG_DSI_LANE_LVDS_LINK_CFG_DUAL BIT(5) /* dual or 2x
> >> single */
> >
> > The bit name here seems a little odd.
> > Bits 5&6 are the DSI channel mode on SN65DSI85, not the LVDS link
> > config (which appears to be reg 0x18 bit 4)
> > DSI_CHANNEL_MODE
> > 00 – Dual-channel DSI receiver
> > 01 – Single channel DSI receiver (default)
> > 10 – Two single channel DSI receivers
> > 11 – Reserved
> > SN65DSI83 and 84 require it to be set to 01. You have that end result,
> > but using an odd register name that only documents one of the 2 bits.
> >
> > Is it worth documenting bit 7 as being the '85 Dual DSI link
> > LEFT_RIGHT_PIXELS flag at the same time? The chip isn't supported in
> > dual DSI mode at present, but defining all the registers up front
> > seems reasonable.
>
> Yes, let's fix those.
>
> [...]
>
> >> + /*
> >> +* Reset the chip, pull EN line low for t_reset=10ms,
> >> +* then high for t_en=1ms.
> >> +*/
> >> + regcache_mark_dirty(ctx->regmap);
> >> + gpiod_set_value(ctx->enable_gpio, 0);
> >> + usleep_range(1, 11000);
> >> + gpiod_set_value(ctx->enable_gpio, 1);
> >> + usleep_range(1000, 1100);
> >> +}
> >
> > Whilst section 6.6 of the SN65DSI84 datasheet does list t_en as 1ms,
> > the initialization sequence listed in table 7.2 states
> > Init seq 3 - Set EN pin to Low
> > Wait 10 ms
> > Init seq 4 - Tie EN pin to High
> > Wait 10 ms
> >
> > with the note that these are "Minimum recommended delay. It is fine to
> > exceed these."
> >
> > Have you had alternate guidance from TI over that delay?
>
> No, I trusted the timing diagrams in section 6.6 of the datasheet. I
> would like to believe those are correct, while the init sequence listing
> might be a copy-paste error.
It's a tough one to call as to which is going to be correct. I just
thought I'd flag it.
> [...]
>
> >> +static void sn65dsi83_enable(struct drm_bridge *bridge)
> >> +{
> >> + struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
> >> + unsigned int pval;
> >> + u16 val;
> >> + int ret;
> >> +
> >> + /* Clear reset, disable PLL */
> >> + regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
> >> + regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
> >> +
> >> + /* Reference clock derived from DSI link clock. */
> >> + regmap_write(ctx->regmap, REG_RC_LVDS_PLL,
> >> +
> >> REG_RC_LVDS_PLL_LVDS_CLK_RANGE(sn65dsi83_get_lvds_range(ctx)) |
> >> + REG_RC_LVDS_PLL_HS_CLK_SRC_DPHY);
> >
> > (Checkpatch whinge for "Alignment should match open parenthesis" on
> > several lines through this function)
>
> Do you have any extra checkpatch settings somewhere ?
> I get this on current next:
>
> linux-2.6$ ./scripts/checkpatch.pl -f drivers/gpu/drm/bridge/ti-sn65dsi83.c
> total: 0 errors, 0 warnings, 625 lines checked
Sorry, yes "./scripts/checkpatch.pl --strict --codespell". Too much
working in the linux-media realms where --strict is required :-/
Dave
> [...]
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On 06.05.21 17:46, Marek Vasut wrote: > On 5/6/21 5:38 PM, Frieder Schrempf wrote: > [...] >> Works on i.MX8MM with SN65DSI84 and a single link LVDS display (1024x600) >> and from my perspective everything else also looks good. Thanks for your >> work! >> >> I have two remarks: >> >> 1. In my test I couldn't get it to work with four DSI lanes enabled (only >> with two) but I'm quite sure that the DSIM driver is to blame as everything >> on the bridge level looks good (also setting the DSI EQ register didn't help >> as you suggested, Marek). > > I suspect there is indeed something with the DSIM going on, I'll keep you > posted if I find something out. > >> 2. When I set MEDIA_BUS_FMT_RGB888_1X7X4_SPWG in the panel driver I get >> distorted colors. I need to use MEDIA_BUS_FMT_RGB888_1X24 to make it work, >> but this is not valid for LVDS. Again I don't think this driver is to blame >> as I can't see where it does anything wrong, but my experience here is very >> limited so I still want to mention it. > > Hmm, in that conversion supposed to happen in this bridge driver or should > MXSFB handle the SPWG pixel format ? Or should the DSIM bridge do something > about that ? As far as I understand it the conversion is already done by the DSI84 without any extra configuration necessary. The only thing that needs to be done is selecting the LVDS output format via CHx_24BPP_MODE and CHx_24BPP_FORMAT1 which the driver currently hardcodes to 24bpp aka MEDIA_BUS_FMT_RGB888_1X7X4_SPWG. I think the DSI input format is always 24bpp aka MEDIA_BUS_FMT_RGB888_1X24. So I wonder where the format actually is evaluated. Could it be that it is passed down to the LCDIF and changes its output format which causes the data passed by DSIM to the DSI84 to already be in the SPWG format? If that's the case we maybe need a way to specify MEDIA_BUS_FMT_RGB888_1X24 as input bus format for the DSI84 so it doesn't pass on the panel's format? Only a wild guess, no idea if it really works like that.
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On 5/6/21 5:38 PM, Frieder Schrempf wrote: [...] Works on i.MX8MM with SN65DSI84 and a single link LVDS display (1024x600) and from my perspective everything else also looks good. Thanks for your work! I have two remarks: 1. In my test I couldn't get it to work with four DSI lanes enabled (only with two) but I'm quite sure that the DSIM driver is to blame as everything on the bridge level looks good (also setting the DSI EQ register didn't help as you suggested, Marek). I suspect there is indeed something with the DSIM going on, I'll keep you posted if I find something out. 2. When I set MEDIA_BUS_FMT_RGB888_1X7X4_SPWG in the panel driver I get distorted colors. I need to use MEDIA_BUS_FMT_RGB888_1X24 to make it work, but this is not valid for LVDS. Again I don't think this driver is to blame as I can't see where it does anything wrong, but my experience here is very limited so I still want to mention it. Hmm, in that conversion supposed to happen in this bridge driver or should MXSFB handle the SPWG pixel format ? Or should the DSIM bridge do something about that ?
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On 05.05.21 12:02, Marek Vasut wrote: > Add driver for TI SN65DSI83 Single-link DSI to Single-link LVDS bridge > and TI SN65DSI84 Single-link DSI to Dual-link or 2x Single-link LVDS > bridge. TI SN65DSI85 is unsupported due to lack of hardware to test on, > but easy to add. > > The driver operates the chip via I2C bus. Currently the LVDS clock are > always derived from DSI clock lane, which is the usual mode of operation. > Support for clock from external oscillator is not implemented, but it is > easy to add if ever needed. Only RGB888 pixel format is implemented, the > LVDS666 is not supported, but could be added if needed. > > Signed-off-by: Marek Vasut > Cc: Douglas Anderson > Cc: Jagan Teki > Cc: Laurent Pinchart > Cc: Linus Walleij > Cc: Loic Poulain > Cc: Philippe Schenker > Cc: Sam Ravnborg > Cc: Stephen Boyd > Cc: Valentin Raevsky > To: dri-devel@lists.freedesktop.org > Reviewed-by: Linus Walleij Works on i.MX8MM with SN65DSI84 and a single link LVDS display (1024x600) and from my perspective everything else also looks good. Thanks for your work! I have two remarks: 1. In my test I couldn't get it to work with four DSI lanes enabled (only with two) but I'm quite sure that the DSIM driver is to blame as everything on the bridge level looks good (also setting the DSI EQ register didn't help as you suggested, Marek). 2. When I set MEDIA_BUS_FMT_RGB888_1X7X4_SPWG in the panel driver I get distorted colors. I need to use MEDIA_BUS_FMT_RGB888_1X24 to make it work, but this is not valid for LVDS. Again I don't think this driver is to blame as I can't see where it does anything wrong, but my experience here is very limited so I still want to mention it. Reviewed-by: Frieder Schrempf Tested-by: Frieder Schrempf > --- > V2: - Use dev_err_probe() > - Set REG_RC_RESET as volatile > - Wait for PLL stabilization by polling REG_RC_LVDS_PLL > - Use ctx->mode = *adj instead of *mode in sn65dsi83_mode_set > - Add tested DSI84 support in dual-link mode > - Correctly set VCOM > - Fill in missing DSI CHB and LVDS CHB bits from DSI84 and DSI85 > datasheets, with that all the reserved bits make far more sense > as the DSI83 and DSI84 seems to be reduced version of DSI85 > V3: - Handle the dual-link LVDS with two port panel or bridge > --- > drivers/gpu/drm/bridge/Kconfig| 10 + > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/ti-sn65dsi83.c | 639 ++ > 3 files changed, 650 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi83.c > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index e83b8ad0d71b..32204c5f25b7 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -278,6 +278,16 @@ config DRM_TI_TFP410 > help > Texas Instruments TFP410 DVI/HDMI Transmitter driver > > +config DRM_TI_SN65DSI83 > + tristate "TI SN65DSI83 and SN65DSI84 DSI to LVDS bridge" > + depends on OF > + select DRM_KMS_HELPER > + select REGMAP_I2C > + select DRM_PANEL > + select DRM_MIPI_DSI > + help > + Texas Instruments SN65DSI83 and SN65DSI84 DSI to LVDS Bridge driver > + > config DRM_TI_SN65DSI86 > tristate "TI SN65DSI86 DSI to eDP bridge" > depends on OF > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index b00f3b2ad572..7bb4c9df0415 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -24,6 +24,7 @@ obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o > obj-$(CONFIG_DRM_TOSHIBA_TC358768) += tc358768.o > obj-$(CONFIG_DRM_TOSHIBA_TC358775) += tc358775.o > obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ > +obj-$(CONFIG_DRM_TI_SN65DSI83) += ti-sn65dsi83.o > obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o > obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o > obj-$(CONFIG_DRM_TI_TPD12S015) += ti-tpd12s015.o > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c > b/drivers/gpu/drm/bridge/ti-sn65dsi83.c > new file mode 100644 > index ..471df09a1c07 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c > @@ -0,0 +1,639 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * TI SN65DSI83,84,85 driver > + * > + * Currently supported: > + * - SN65DSI83 > + * = 1x Single-link DSI ~ 1x Single-link LVDS > + * - Supported > + * - Single-link LVDS mode tested > + * - SN65DSI84 > + * = 1x Single-link DSI ~ 2x Single-link or 1x Dual-link LVDS > + * - Supported > + * - Dual-link LVDS mode tested > + * - 2x Single-link LVDS mode unsupported > + * (should be easy to add by someone who has the HW) > + * - SN65DSI85 > + * = 2x Single-link or 1x Dual-link DSI ~ 2x Single-link or 1x Dual-link > LVDS > + * - Unsupported > + * (should be easy to add by someone who has the HW) > + * > + * Copyright (C) 2021 Marek Vasut > + * > + * Based on previous work of: > + * Valentin Raevsky > + * Philippe
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On 5/6/21 3:03 PM, Laurent Pinchart wrote: Hi Marek, Hi, On Thu, May 06, 2021 at 02:48:11PM +0200, Marek Vasut wrote: On 5/6/21 11:45 AM, Dave Stevenson wrote: Hi Marek Hi, I'm taking an interest as there are a number of Raspberry Pi users trying to get this chip up and running (not there quite yet). A couple of fairly minor comments Is there any readily available display unit / expansion board with this chip ? For what it's worth, I have a board with a Raspberry Pi CM4 and a SN65DSI83. It's a customer design, not an off-the-shelf part I'm afraid, but I plan to eventually test your driver on it. Perfect
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
Hi Marek,
On Thu, May 06, 2021 at 02:48:11PM +0200, Marek Vasut wrote:
> On 5/6/21 11:45 AM, Dave Stevenson wrote:
> > Hi Marek
>
> Hi,
>
> > I'm taking an interest as there are a number of Raspberry Pi users
> > trying to get this chip up and running (not there quite yet).
> > A couple of fairly minor comments
>
> Is there any readily available display unit / expansion board with this
> chip ?
For what it's worth, I have a board with a Raspberry Pi CM4 and a
SN65DSI83. It's a customer design, not an off-the-shelf part I'm afraid,
but I plan to eventually test your driver on it.
> [...]
>
> >> +#define REG_DSI_LANE 0x10
> >> +#define REG_DSI_LANE_LVDS_LINK_CFG_DUAL BIT(5) /* dual or 2x
> >> single */
> >
> > The bit name here seems a little odd.
> > Bits 5&6 are the DSI channel mode on SN65DSI85, not the LVDS link
> > config (which appears to be reg 0x18 bit 4)
> > DSI_CHANNEL_MODE
> > 00 – Dual-channel DSI receiver
> > 01 – Single channel DSI receiver (default)
> > 10 – Two single channel DSI receivers
> > 11 – Reserved
> > SN65DSI83 and 84 require it to be set to 01. You have that end result,
> > but using an odd register name that only documents one of the 2 bits.
> >
> > Is it worth documenting bit 7 as being the '85 Dual DSI link
> > LEFT_RIGHT_PIXELS flag at the same time? The chip isn't supported in
> > dual DSI mode at present, but defining all the registers up front
> > seems reasonable.
>
> Yes, let's fix those.
>
> [...]
>
> >> + /*
> >> +* Reset the chip, pull EN line low for t_reset=10ms,
> >> +* then high for t_en=1ms.
> >> +*/
> >> + regcache_mark_dirty(ctx->regmap);
> >> + gpiod_set_value(ctx->enable_gpio, 0);
> >> + usleep_range(1, 11000);
> >> + gpiod_set_value(ctx->enable_gpio, 1);
> >> + usleep_range(1000, 1100);
> >> +}
> >
> > Whilst section 6.6 of the SN65DSI84 datasheet does list t_en as 1ms,
> > the initialization sequence listed in table 7.2 states
> > Init seq 3 - Set EN pin to Low
> > Wait 10 ms
> > Init seq 4 - Tie EN pin to High
> > Wait 10 ms
> >
> > with the note that these are "Minimum recommended delay. It is fine to
> > exceed these."
> >
> > Have you had alternate guidance from TI over that delay?
>
> No, I trusted the timing diagrams in section 6.6 of the datasheet. I
> would like to believe those are correct, while the init sequence listing
> might be a copy-paste error.
>
> [...]
>
> >> +static void sn65dsi83_enable(struct drm_bridge *bridge)
> >> +{
> >> + struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
> >> + unsigned int pval;
> >> + u16 val;
> >> + int ret;
> >> +
> >> + /* Clear reset, disable PLL */
> >> + regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
> >> + regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
> >> +
> >> + /* Reference clock derived from DSI link clock. */
> >> + regmap_write(ctx->regmap, REG_RC_LVDS_PLL,
> >> +
> >> REG_RC_LVDS_PLL_LVDS_CLK_RANGE(sn65dsi83_get_lvds_range(ctx)) |
> >> + REG_RC_LVDS_PLL_HS_CLK_SRC_DPHY);
> >
> > (Checkpatch whinge for "Alignment should match open parenthesis" on
> > several lines through this function)
>
> Do you have any extra checkpatch settings somewhere ?
> I get this on current next:
>
> linux-2.6$ ./scripts/checkpatch.pl -f drivers/gpu/drm/bridge/ti-sn65dsi83.c
> total: 0 errors, 0 warnings, 625 lines checked
>
> [...]
--
Regards,
Laurent Pinchart
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On 5/6/21 11:45 AM, Dave Stevenson wrote:
Hi Marek
Hi,
I'm taking an interest as there are a number of Raspberry Pi users
trying to get this chip up and running (not there quite yet).
A couple of fairly minor comments
Is there any readily available display unit / expansion board with this
chip ?
[...]
+#define REG_DSI_LANE 0x10
+#define REG_DSI_LANE_LVDS_LINK_CFG_DUAL BIT(5) /* dual or 2x single */
The bit name here seems a little odd.
Bits 5&6 are the DSI channel mode on SN65DSI85, not the LVDS link
config (which appears to be reg 0x18 bit 4)
DSI_CHANNEL_MODE
00 – Dual-channel DSI receiver
01 – Single channel DSI receiver (default)
10 – Two single channel DSI receivers
11 – Reserved
SN65DSI83 and 84 require it to be set to 01. You have that end result,
but using an odd register name that only documents one of the 2 bits.
Is it worth documenting bit 7 as being the '85 Dual DSI link
LEFT_RIGHT_PIXELS flag at the same time? The chip isn't supported in
dual DSI mode at present, but defining all the registers up front
seems reasonable.
Yes, let's fix those.
[...]
+ /*
+* Reset the chip, pull EN line low for t_reset=10ms,
+* then high for t_en=1ms.
+*/
+ regcache_mark_dirty(ctx->regmap);
+ gpiod_set_value(ctx->enable_gpio, 0);
+ usleep_range(1, 11000);
+ gpiod_set_value(ctx->enable_gpio, 1);
+ usleep_range(1000, 1100);
+}
Whilst section 6.6 of the SN65DSI84 datasheet does list t_en as 1ms,
the initialization sequence listed in table 7.2 states
Init seq 3 - Set EN pin to Low
Wait 10 ms
Init seq 4 - Tie EN pin to High
Wait 10 ms
with the note that these are "Minimum recommended delay. It is fine to
exceed these."
Have you had alternate guidance from TI over that delay?
No, I trusted the timing diagrams in section 6.6 of the datasheet. I
would like to believe those are correct, while the init sequence listing
might be a copy-paste error.
[...]
+static void sn65dsi83_enable(struct drm_bridge *bridge)
+{
+ struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
+ unsigned int pval;
+ u16 val;
+ int ret;
+
+ /* Clear reset, disable PLL */
+ regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
+ regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
+
+ /* Reference clock derived from DSI link clock. */
+ regmap_write(ctx->regmap, REG_RC_LVDS_PLL,
+ REG_RC_LVDS_PLL_LVDS_CLK_RANGE(sn65dsi83_get_lvds_range(ctx)) |
+ REG_RC_LVDS_PLL_HS_CLK_SRC_DPHY);
(Checkpatch whinge for "Alignment should match open parenthesis" on
several lines through this function)
Do you have any extra checkpatch settings somewhere ?
I get this on current next:
linux-2.6$ ./scripts/checkpatch.pl -f drivers/gpu/drm/bridge/ti-sn65dsi83.c
total: 0 errors, 0 warnings, 625 lines checked
[...]
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
Hi Marek I'm taking an interest as there are a number of Raspberry Pi users trying to get this chip up and running (not there quite yet). A couple of fairly minor comments On Wed, 5 May 2021 at 11:03, Marek Vasut wrote: > > Add driver for TI SN65DSI83 Single-link DSI to Single-link LVDS bridge > and TI SN65DSI84 Single-link DSI to Dual-link or 2x Single-link LVDS > bridge. TI SN65DSI85 is unsupported due to lack of hardware to test on, > but easy to add. > > The driver operates the chip via I2C bus. Currently the LVDS clock are > always derived from DSI clock lane, which is the usual mode of operation. > Support for clock from external oscillator is not implemented, but it is > easy to add if ever needed. Only RGB888 pixel format is implemented, the > LVDS666 is not supported, but could be added if needed. > > Signed-off-by: Marek Vasut > Cc: Douglas Anderson > Cc: Jagan Teki > Cc: Laurent Pinchart > Cc: Linus Walleij > Cc: Loic Poulain > Cc: Philippe Schenker > Cc: Sam Ravnborg > Cc: Stephen Boyd > Cc: Valentin Raevsky > To: dri-devel@lists.freedesktop.org > --- > V2: - Use dev_err_probe() > - Set REG_RC_RESET as volatile > - Wait for PLL stabilization by polling REG_RC_LVDS_PLL > - Use ctx->mode = *adj instead of *mode in sn65dsi83_mode_set > - Add tested DSI84 support in dual-link mode > - Correctly set VCOM > - Fill in missing DSI CHB and LVDS CHB bits from DSI84 and DSI85 > datasheets, with that all the reserved bits make far more sense > as the DSI83 and DSI84 seems to be reduced version of DSI85 > V3: - Handle the dual-link LVDS with two port panel or bridge > --- > drivers/gpu/drm/bridge/Kconfig| 10 + > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/ti-sn65dsi83.c | 639 ++ > 3 files changed, 650 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi83.c > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index e83b8ad0d71b..32204c5f25b7 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -278,6 +278,16 @@ config DRM_TI_TFP410 > help > Texas Instruments TFP410 DVI/HDMI Transmitter driver > > +config DRM_TI_SN65DSI83 > + tristate "TI SN65DSI83 and SN65DSI84 DSI to LVDS bridge" > + depends on OF > + select DRM_KMS_HELPER > + select REGMAP_I2C > + select DRM_PANEL > + select DRM_MIPI_DSI > + help > + Texas Instruments SN65DSI83 and SN65DSI84 DSI to LVDS Bridge driver > + > config DRM_TI_SN65DSI86 > tristate "TI SN65DSI86 DSI to eDP bridge" > depends on OF > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index b00f3b2ad572..7bb4c9df0415 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -24,6 +24,7 @@ obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o > obj-$(CONFIG_DRM_TOSHIBA_TC358768) += tc358768.o > obj-$(CONFIG_DRM_TOSHIBA_TC358775) += tc358775.o > obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ > +obj-$(CONFIG_DRM_TI_SN65DSI83) += ti-sn65dsi83.o > obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o > obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o > obj-$(CONFIG_DRM_TI_TPD12S015) += ti-tpd12s015.o > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c > b/drivers/gpu/drm/bridge/ti-sn65dsi83.c > new file mode 100644 > index ..471df09a1c07 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c > @@ -0,0 +1,639 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * TI SN65DSI83,84,85 driver > + * > + * Currently supported: > + * - SN65DSI83 > + * = 1x Single-link DSI ~ 1x Single-link LVDS > + * - Supported > + * - Single-link LVDS mode tested > + * - SN65DSI84 > + * = 1x Single-link DSI ~ 2x Single-link or 1x Dual-link LVDS > + * - Supported > + * - Dual-link LVDS mode tested > + * - 2x Single-link LVDS mode unsupported > + * (should be easy to add by someone who has the HW) > + * - SN65DSI85 > + * = 2x Single-link or 1x Dual-link DSI ~ 2x Single-link or 1x Dual-link > LVDS > + * - Unsupported > + * (should be easy to add by someone who has the HW) > + * > + * Copyright (C) 2021 Marek Vasut > + * > + * Based on previous work of: > + * Valentin Raevsky > + * Philippe Schenker > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/* ID registers */ > +#define REG_ID(n) (0x00 + (n)) > +/* Reset and clock registers */ > +#define REG_RC_RESET 0x09 > +#define REG_RC_RESET_SOFT_RESET BIT(0) > +#define REG_RC_LVDS_PLL0x0a > +#define REG_RC_LVDS_PLL_PLL_EN_STAT BIT(7) > +#define REG_RC_LVDS_PLL_LVDS_CLK_RANGE(n) (((n) & 0x7) << 1) > +#define
Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
On Wed, May 5, 2021 at 12:03 PM Marek Vasut wrote: > Add driver for TI SN65DSI83 Single-link DSI to Single-link LVDS bridge > and TI SN65DSI84 Single-link DSI to Dual-link or 2x Single-link LVDS > bridge. TI SN65DSI85 is unsupported due to lack of hardware to test on, > but easy to add. > > The driver operates the chip via I2C bus. Currently the LVDS clock are > always derived from DSI clock lane, which is the usual mode of operation. > Support for clock from external oscillator is not implemented, but it is > easy to add if ever needed. Only RGB888 pixel format is implemented, the > LVDS666 is not supported, but could be added if needed. > > Signed-off-by: Marek Vasut > Cc: Douglas Anderson > Cc: Jagan Teki > Cc: Laurent Pinchart > Cc: Linus Walleij > Cc: Loic Poulain > Cc: Philippe Schenker > Cc: Sam Ravnborg > Cc: Stephen Boyd > Cc: Valentin Raevsky > To: dri-devel@lists.freedesktop.org This looks very nice to me, despite the complex nature of the driver it is a bliss to read this code with all nice comments and explanations. I don't know the gory details as well as the bridge maintainers but from my helicopter perspective, hats off. Reviewed-by: Linus Walleij Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver
Add driver for TI SN65DSI83 Single-link DSI to Single-link LVDS bridge and TI SN65DSI84 Single-link DSI to Dual-link or 2x Single-link LVDS bridge. TI SN65DSI85 is unsupported due to lack of hardware to test on, but easy to add. The driver operates the chip via I2C bus. Currently the LVDS clock are always derived from DSI clock lane, which is the usual mode of operation. Support for clock from external oscillator is not implemented, but it is easy to add if ever needed. Only RGB888 pixel format is implemented, the LVDS666 is not supported, but could be added if needed. Signed-off-by: Marek Vasut Cc: Douglas Anderson Cc: Jagan Teki Cc: Laurent Pinchart Cc: Linus Walleij Cc: Loic Poulain Cc: Philippe Schenker Cc: Sam Ravnborg Cc: Stephen Boyd Cc: Valentin Raevsky To: dri-devel@lists.freedesktop.org --- V2: - Use dev_err_probe() - Set REG_RC_RESET as volatile - Wait for PLL stabilization by polling REG_RC_LVDS_PLL - Use ctx->mode = *adj instead of *mode in sn65dsi83_mode_set - Add tested DSI84 support in dual-link mode - Correctly set VCOM - Fill in missing DSI CHB and LVDS CHB bits from DSI84 and DSI85 datasheets, with that all the reserved bits make far more sense as the DSI83 and DSI84 seems to be reduced version of DSI85 V3: - Handle the dual-link LVDS with two port panel or bridge --- drivers/gpu/drm/bridge/Kconfig| 10 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/ti-sn65dsi83.c | 639 ++ 3 files changed, 650 insertions(+) create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi83.c diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index e83b8ad0d71b..32204c5f25b7 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -278,6 +278,16 @@ config DRM_TI_TFP410 help Texas Instruments TFP410 DVI/HDMI Transmitter driver +config DRM_TI_SN65DSI83 + tristate "TI SN65DSI83 and SN65DSI84 DSI to LVDS bridge" + depends on OF + select DRM_KMS_HELPER + select REGMAP_I2C + select DRM_PANEL + select DRM_MIPI_DSI + help + Texas Instruments SN65DSI83 and SN65DSI84 DSI to LVDS Bridge driver + config DRM_TI_SN65DSI86 tristate "TI SN65DSI86 DSI to eDP bridge" depends on OF diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index b00f3b2ad572..7bb4c9df0415 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -24,6 +24,7 @@ obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o obj-$(CONFIG_DRM_TOSHIBA_TC358768) += tc358768.o obj-$(CONFIG_DRM_TOSHIBA_TC358775) += tc358775.o obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ +obj-$(CONFIG_DRM_TI_SN65DSI83) += ti-sn65dsi83.o obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o obj-$(CONFIG_DRM_TI_TPD12S015) += ti-tpd12s015.o diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c new file mode 100644 index ..471df09a1c07 --- /dev/null +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c @@ -0,0 +1,639 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * TI SN65DSI83,84,85 driver + * + * Currently supported: + * - SN65DSI83 + * = 1x Single-link DSI ~ 1x Single-link LVDS + * - Supported + * - Single-link LVDS mode tested + * - SN65DSI84 + * = 1x Single-link DSI ~ 2x Single-link or 1x Dual-link LVDS + * - Supported + * - Dual-link LVDS mode tested + * - 2x Single-link LVDS mode unsupported + * (should be easy to add by someone who has the HW) + * - SN65DSI85 + * = 2x Single-link or 1x Dual-link DSI ~ 2x Single-link or 1x Dual-link LVDS + * - Unsupported + * (should be easy to add by someone who has the HW) + * + * Copyright (C) 2021 Marek Vasut + * + * Based on previous work of: + * Valentin Raevsky + * Philippe Schenker + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +/* ID registers */ +#define REG_ID(n) (0x00 + (n)) +/* Reset and clock registers */ +#define REG_RC_RESET 0x09 +#define REG_RC_RESET_SOFT_RESET BIT(0) +#define REG_RC_LVDS_PLL0x0a +#define REG_RC_LVDS_PLL_PLL_EN_STAT BIT(7) +#define REG_RC_LVDS_PLL_LVDS_CLK_RANGE(n) (((n) & 0x7) << 1) +#define REG_RC_LVDS_PLL_HS_CLK_SRC_DPHY BIT(0) +#define REG_RC_DSI_CLK 0x0b +#define REG_RC_DSI_CLK_DSI_CLK_DIVIDER(n) (((n) & 0x1f) << 3) +#define REG_RC_DSI_CLK_REFCLK_MULTIPLIER(n) ((n) & 0x3) +#define REG_RC_PLL_EN 0x0d +#define REG_RC_PLL_EN_PLL_EN BIT(0) +/* DSI registers */ +#define REG_DSI_LANE 0x10 +#define REG_DSI_LANE_LVDS_LINK_CFG_DUAL BIT(5) /* dual or 2x single */ +#define
