Re: [PATCH v2] drm/panel: Enable DSC and CMD mode for Visionox VTDR6130 panel

[Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>]

On 10/9/2023 3:32 AM, neil.armstrong@xxxxxxxxxx wrote:
> Hi Paloma,
>
> On 28/07/2023 03:26, Paloma Arellano wrote:
> > Enable display compression (DSC v1.2) and CMD mode for 1080x2400 Visionox
> > VTDR6130 AMOLED DSI panel. In addition, this patch will set the default
> > to command mode with DSC enabled.
> >
> > Note: This patch has only been validated DSC over command mode as DSC 
> > over
> > video mode has never been validated for the MSM driver before.
>
> I was able to test this on the QRD8550 on top of v6.6-rc5, display works 
> fine,
> but when runnning:
> # modetest -r -v
> <snip>
> setting mode 1080x2400-144.00Hz on connectors 32, crtc 95
> failed to set gamma: Function not implemented
> freq: 74.22Hz
> freq: 73.09Hz
> freq: 72.48Hz
>
> We get a correct 144Hz vsync test in video mode.
>
> Do you know why this happens ?

Hi Neil,

Thanks for reporting this.

FWIW, I saw a slight drop in FPS (from 120 to 90Hz) when enabling DSC 
for the r66451 command mode panel though the drop reported here seems 
more drastic.

I'll try recreating this on SM8550 MTP and investigate it.

Thanks,

Jessica Zhang

> Neil
>
> > Depends on: "Add prepare_prev_first flag to Visionox VTDR6130" [1]
> >
> > Changes since v1:
> >   - Changed from email address
> >
> > [1] https://patchwork.freedesktop.org/series/121337/
> >
> > Suggested-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>
> > Signed-off-by: Paloma Arellano <quic_parellan@xxxxxxxxxxx>
> > ---
> >   .../gpu/drm/panel/panel-visionox-vtdr6130.c   | 77 ++++++++++++++++++-
> >   1 file changed, 73 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c 
> > b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
> > index e1363e128e7e..5658d39a3a6b 100644
> > --- a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
> > +++ b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
> > @@ -9,6 +9,7 @@
> >   #include <linux/of.h>
> >   #include <drm/display/drm_dsc.h>
> > +#include <drm/display/drm_dsc_helper.h>
> >   #include <drm/drm_mipi_dsi.h>
> >   #include <drm/drm_modes.h>
> >   #include <drm/drm_panel.h>
> > @@ -20,7 +21,8 @@ struct visionox_vtdr6130 {
> >       struct mipi_dsi_device *dsi;
> >       struct gpio_desc *reset_gpio;
> >       struct regulator_bulk_data supplies[3];
> > -    bool prepared;
> > +    bool prepared, enabled;
> > +    bool video_mode;
> >   };
> >   static inline struct visionox_vtdr6130 *to_visionox_vtdr6130(struct 
> > drm_panel *panel)
> > @@ -50,12 +52,18 @@ static int visionox_vtdr6130_on(struct 
> > visionox_vtdr6130 *ctx)
> >       if (ret)
> >           return ret;
> > +    mipi_dsi_dcs_write_seq(dsi, 0x03, 0x01);
> >       mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x20);
> >       mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS, 
> > 0x00, 0x00);
> >       mipi_dsi_dcs_write_seq(dsi, 0x59, 0x09);
> >       mipi_dsi_dcs_write_seq(dsi, 0x6c, 0x01);
> >       mipi_dsi_dcs_write_seq(dsi, 0x6d, 0x00);
> > -    mipi_dsi_dcs_write_seq(dsi, 0x6f, 0x01);
> > +
> > +    if (ctx->video_mode)
> > +        mipi_dsi_dcs_write_seq(dsi, 0x6f, 0x01);
> > +    else
> > +        mipi_dsi_dcs_write_seq(dsi, 0x6f, 0x02);
> > +
> >       mipi_dsi_dcs_write_seq(dsi, 0x70,
> >                      0x12, 0x00, 0x00, 0xab, 0x30, 0x80, 0x09, 0x60, 
> > 0x04,
> >                      0x38, 0x00, 0x28, 0x02, 0x1c, 0x02, 0x1c, 0x02, 
> > 0x00,
> > @@ -214,6 +222,42 @@ static const struct drm_display_mode 
> > visionox_vtdr6130_mode = {
> >       .height_mm = 157,
> >   };
> > +static int visionox_vtdr6130_enable(struct drm_panel *panel)
> > +{
> > +    struct visionox_vtdr6130 *ctx = to_visionox_vtdr6130(panel);
> > +    struct mipi_dsi_device *dsi = ctx->dsi;
> > +    struct drm_dsc_picture_parameter_set pps;
> > +    int ret;
> > +
> > +    if (ctx->enabled)
> > +        return 0;
> > +
> > +    if (!dsi->dsc) {
> > +        dev_err(&dsi->dev, "DSC not attached to DSI\n");
> > +        return -ENODEV;
> > +    }
> > +
> > +    drm_dsc_pps_payload_pack(&pps, dsi->dsc);
> > +    ret = mipi_dsi_picture_parameter_set(dsi, &pps);
> > +    if (ret) {
> > +        dev_err(&dsi->dev, "Failed to set PPS\n");
> > +        return ret;
> > +    }
> > +
> > +    ctx->enabled = true;
> > +
> > +    return 0;
> > +}
> > +
> > +static int visionox_vtdr6130_disable(struct drm_panel *panel)
> > +{
> > +    struct visionox_vtdr6130 *ctx = to_visionox_vtdr6130(panel);
> > +
> > +    ctx->enabled = false;
> > +
> > +    return 0;
> > +}
> > +
> >   static int visionox_vtdr6130_get_modes(struct drm_panel *panel,
> >                          struct drm_connector *connector)
> >   {
> > @@ -237,6 +281,8 @@ static const struct drm_panel_funcs 
> > visionox_vtdr6130_panel_funcs = {
> >       .prepare = visionox_vtdr6130_prepare,
> >       .unprepare = visionox_vtdr6130_unprepare,
> >       .get_modes = visionox_vtdr6130_get_modes,
> > +    .enable = visionox_vtdr6130_enable,
> > +    .disable = visionox_vtdr6130_disable,
> >   };
> >   static int visionox_vtdr6130_bl_update_status(struct 
> > backlight_device *bl)
> > @@ -269,11 +315,31 @@ static int visionox_vtdr6130_probe(struct 
> > mipi_dsi_device *dsi)
> >   {
> >       struct device *dev = &dsi->dev;
> >       struct visionox_vtdr6130 *ctx;
> > +    struct drm_dsc_config *dsc;
> >       int ret;
> >       ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> >       if (!ctx)
> >           return -ENOMEM;
> > +
> > +    ctx->video_mode = of_property_read_bool(dev->of_node, 
> > "enforce-video-mode");
> > +
> > +    dsc = devm_kzalloc(dev, sizeof(*dsc), GFP_KERNEL);
> > +    if (!dsc)
> > +        return -ENOMEM;
> > +
> > +    /* Set DSC params */
> > +    dsc->dsc_version_major = 0x1;
> > +    dsc->dsc_version_minor = 0x2;
> > +
> > +    dsc->slice_height = 40;
> > +    dsc->slice_width = 540;
> > +    dsc->slice_count = 2;
> > +    dsc->bits_per_component = 8;
> > +    dsc->bits_per_pixel = 8 << 4;
> > +    dsc->block_pred_enable = true;
> > +
> > +    dsi->dsc = dsc;
> >       ctx->supplies[0].supply = "vddio";
> >       ctx->supplies[1].supply = "vci";
> > @@ -294,8 +360,11 @@ static int visionox_vtdr6130_probe(struct 
> > mipi_dsi_device *dsi)
> >       dsi->lanes = 4;
> >       dsi->format = MIPI_DSI_FMT_RGB888;
> > -    dsi->mode_flags = MIPI_DSI_MODE_VIDEO | 
> > MIPI_DSI_MODE_NO_EOT_PACKET |
> > -              MIPI_DSI_CLOCK_NON_CONTINUOUS;
> > +
> > +    dsi->mode_flags = MIPI_DSI_MODE_NO_EOT_PACKET | 
> > MIPI_DSI_CLOCK_NON_CONTINUOUS;
> > +    if (ctx->video_mode)
> > +        dsi->mode_flags |= MIPI_DSI_MODE_VIDEO;
> > +
> >       ctx->panel.prepare_prev_first = true;
> >       drm_panel_init(&ctx->panel, dev, &visionox_vtdr6130_panel_funcs,