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(10000, 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
[...]
