From: Sam Ravnborg <sam@ravnborg.org>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: devicetree@vger.kernel.org,
Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
dri-devel@lists.freedesktop.org, Rob Herring <robh+dt@kernel.org>,
Thierry Reding <thierry.reding@gmail.com>,
Maxime Ripard <mripard@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>
Subject: Re: [PATCH 7/7] drm/panel: sitronix-st7789v: Check display ID
Date: Sat, 10 Jun 2023 22:45:25 +0200 [thread overview]
Message-ID: <20230610204525.GA1042549@ravnborg.org> (raw)
In-Reply-To: <20230609145951.853533-8-miquel.raynal@bootlin.com>
Hi Miquel,
On Fri, Jun 09, 2023 at 04:59:51PM +0200, Miquel Raynal wrote:
> A very basic debugging rule when a device is connected for the first
> time is to access a read-only register which contains known data in
> order to ensure the communication protocol is properly working. This
> driver lacked any read helper which is often a critical peace for
> fastening bring-ups.
>
> Add a read helper and use it to verify the communication with the panel
> is working as soon as possible in order to fail early if this is not the
> case.
The read helper seems like a log of general boiler plate code.
I briefly looked into the use of regmap for the spi communication,
but it did not look like it supported the bit9 stuff.
I did not stare enough to add a reviewd by, but the approach is fine
and it is good to detech issues early.
Acked-by: Sam Ravnborg <sam@ravnborg.org>
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> .../gpu/drm/panel/panel-sitronix-st7789v.c | 78 +++++++++++++++++++
> 1 file changed, 78 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> index 7de192a3a8aa..fb21d52a7a63 100644
> --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> @@ -113,6 +113,9 @@
> return val; \
> } while (0)
>
> +#define ST7789V_IDS { 0x85, 0x85, 0x52 }
> +#define ST7789V_IDS_SIZE 3
> +
> struct st7789v_panel_info {
> const struct drm_display_mode *display_mode;
> u16 width_mm;
> @@ -165,6 +168,77 @@ static int st7789v_write_data(struct st7789v *ctx, u8 cmd)
> return st7789v_spi_write(ctx, ST7789V_DATA, cmd);
> }
>
> +static int st7789v_read_data(struct st7789v *ctx, u8 cmd, u8 *buf,
> + unsigned int len)
> +{
> + struct spi_transfer xfer[2] = { };
> + struct spi_message msg;
> + u16 txbuf = ((ST7789V_COMMAND & 1) << 8) | cmd;
> + u16 rxbuf[4] = {};
> + u8 bit9 = 0;
> + int ret, i;
> +
> + switch (len) {
> + case 1:
> + case 3:
> + case 4:
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + spi_message_init(&msg);
> +
> + xfer[0].tx_buf = &txbuf;
> + xfer[0].len = sizeof(txbuf);
> + spi_message_add_tail(&xfer[0], &msg);
> +
> + xfer[1].rx_buf = rxbuf;
> + xfer[1].len = len * 2;
> + spi_message_add_tail(&xfer[1], &msg);
> +
> + ret = spi_sync(ctx->spi, &msg);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < len; i++) {
> + buf[i] = rxbuf[i] >> i | (bit9 << (9 - i));
> + if (i)
> + bit9 = rxbuf[i] & GENMASK(i - 1, 0);
> + }
> +
> + return 0;
> +}
> +
> +static int st7789v_check_id(struct drm_panel *panel)
> +{
> + const u8 st7789v_ids[ST7789V_IDS_SIZE] = ST7789V_IDS;
> + struct st7789v *ctx = panel_to_st7789v(panel);
> + bool invalid_ids = false;
> + int ret, i;
> + u8 ids[3];
> +
> + ret = st7789v_read_data(ctx, MIPI_DCS_GET_DISPLAY_ID, ids, ST7789V_IDS_SIZE);
> + if (ret) {
> + dev_err(panel->dev, "Failed to read IDs\n");
> + return ret;
> + }
> +
> + for (i = 0; i < ST7789V_IDS_SIZE; i++) {
> + if (ids[i] != st7789v_ids[i]) {
> + invalid_ids = true;
> + break;
> + }
> + }
> +
> + if (invalid_ids) {
> + dev_err(panel->dev, "Unrecognized panel IDs");
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> static const struct drm_display_mode default_mode = {
> .clock = 7000,
> .hdisplay = 240,
> @@ -237,6 +311,10 @@ static int st7789v_prepare(struct drm_panel *panel)
> gpiod_set_value(ctx->reset, 0);
> msleep(120);
>
> + ret = st7789v_check_id(panel);
> + if (ret)
> + return ret;
> +
> ST7789V_TEST(ret, st7789v_write_command(ctx, MIPI_DCS_EXIT_SLEEP_MODE));
>
> /* We need to wait 120ms after a sleep out command */
> --
> 2.34.1
next prev parent reply other threads:[~2023-06-10 20:45 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-09 14:59 [PATCH 0/7] drm/panel: sitronix-st7789v: Support ET028013DMA panel Miquel Raynal
2023-06-09 14:59 ` [PATCH 1/7] drm/panel: sitronix-st7789v: Prevent core spi warnings Miquel Raynal
2023-06-10 20:06 ` Sam Ravnborg
2023-06-13 6:15 ` Michael Riesch
2023-06-13 6:56 ` Miquel Raynal
2023-06-14 23:22 ` Sebastian Reichel
2023-06-15 5:43 ` Sam Ravnborg
2023-06-15 12:58 ` Sebastian Reichel
2023-06-09 14:59 ` [PATCH 2/7] drm/panel: sitronix-st7789v: Use 9 bits per spi word by default Miquel Raynal
2023-06-10 20:09 ` Sam Ravnborg
2023-06-09 14:59 ` [PATCH 3/7] drm/panel: sitronix-st7789v: Specify the expected bus format Miquel Raynal
2023-06-10 20:12 ` Sam Ravnborg
2023-06-16 9:56 ` Miquel Raynal
2023-06-12 8:44 ` Maxime Ripard
2023-06-09 14:59 ` [PATCH 4/7] drm/panel: sitronix-st7789v: Use platform data Miquel Raynal
2023-06-10 20:15 ` Sam Ravnborg
2023-06-09 14:59 ` [PATCH 5/7] dt-bindings: display: st7789v: Add the edt, et028013dma panel compatible Miquel Raynal
2023-06-09 14:59 ` [PATCH 6/7] drm/panel: sitronix-st7789v: Add EDT ET028013DMA panel support Miquel Raynal
2023-06-10 20:22 ` Sam Ravnborg
2023-06-12 8:51 ` Maxime Ripard
2023-06-16 10:01 ` Miquel Raynal
2023-06-09 14:59 ` [PATCH 7/7] drm/panel: sitronix-st7789v: Check display ID Miquel Raynal
2023-06-10 20:45 ` Sam Ravnborg [this message]
2023-06-14 23:27 ` Sebastian Reichel
2023-06-16 10:13 ` Miquel Raynal
2023-06-16 13:38 ` Sebastian Reichel
2023-06-13 6:25 ` Michael Riesch
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230610204525.GA1042549@ravnborg.org \
--to=sam@ravnborg.org \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=krzk+dt@kernel.org \
--cc=miquel.raynal@bootlin.com \
--cc=mripard@kernel.org \
--cc=robh+dt@kernel.org \
--cc=thierry.reding@gmail.com \
--cc=thomas.petazzoni@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).