Re: [PATCH 2/2] drm/panel: Add driver for Visionox r66451 panel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 22/05/2023 12:10, Neil Armstrong wrote:
On 18/05/2023 02:19, Dmitry Baryshkov wrote:
On 16/05/2023 23:20, Jessica Zhang wrote:
Add support for the 1080x2340 Visionox R66451 AMOLED DSI panel that
comes with the Qualcomm HDK8350 display expansion pack.

The panel enables display compression (DSC v1.2) by default.

Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>
---
  drivers/gpu/drm/panel/Kconfig                 |   8 +
  drivers/gpu/drm/panel/Makefile                |   1 +
  drivers/gpu/drm/panel/panel-visionox-r66451.c | 395 ++++++++++++++++++++++++++
  3 files changed, 404 insertions(+)

[skipped]

+
+    ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
+    if (ret < 0) {
+        dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
+        return ret;
+    }
+    msleep(120);
+
+    ret = mipi_dsi_dcs_set_display_on(dsi);

Should the mipi_dsi_dcs_set_display_on() (and maybe exit sleep mode) be a a part of _enable()? Correspondingly _off should IMO be a part of _disable callback().

Nop, mipi_dsi_dcs_set_display_on() should be part of prepare, to be called *before* the video stream starts, enable is called after in the atomic modeset chain,

To summarize our discussion: some controllers do not support sending commands in non-LPM mode. Thus the panel driver should send all commands before the DSI host switches from LPM to VIDEO mode.

Is this corret?



+    if (ret < 0) {
+        dev_err(dev, "Failed on set display on: %d\n", ret);
+        return ret;
+    }
+    msleep(20);


Do we need to add here the following line?

dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;


No, it will be changed by the DSI core.

Hmm, where?



+
+    return 0;
+}

[skipped]

+static int visionox_r66451_bl_update_status(struct backlight_device *bl)
+{
+    struct mipi_dsi_device *dsi = bl_get_data(bl);
+    u16 brightness = backlight_get_brightness(bl);
+
+    return mipi_dsi_dcs_set_display_brightness(dsi, cpu_to_le16(brightness));

mipi_dsi_dcs_set_display_brightness() already converts the brightness, so you don't need cpu_to_le16 here.


The _large variant must be used instead.

But cpu_to_le16 is NOP, isn't it?

--
With best wishes
Dmitry




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux