[Freedreno] [PATCH v2] drm/panel: Enable DSC and CMD mode for Visionox VTDR6130 panel
Dmitry Baryshkov
dmitry.baryshkov at linaro.org
Fri Jul 28 23:21:22 UTC 2023
On 29/07/2023 00:44, Jessica Zhang wrote:
>
>
> On 7/28/2023 2:37 AM, Dmitry Baryshkov wrote:
>> On Fri, 28 Jul 2023 at 04:26, Paloma Arellano
>> <quic_parellan at quicinc.com> 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.
>>>
>>> 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 at quicinc.com>
>>> Signed-off-by: Paloma Arellano <quic_parellan at quicinc.com>
>>> ---
>>> .../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;
>>> + }
>>
>> The error message is misleading. Also, if you don't want to enable DSC
>> for the video mode, this will break.
>>
>>> +
>>> + 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;
>>
>> Do we need this refcount just for PPS upload? What will happen if PPS
>> is uploaded several times?
>>
>>> +
>>> + 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");
>>
>> Please also add a DT bindings patch.
>>
>>> +
>>> + dsc = devm_kzalloc(dev, sizeof(*dsc), GFP_KERNEL);
>>> + if (!dsc)
>>> + return -ENOMEM;
>>
>> You can add struct drm_dsc_config to struct visionox_vtdr6130 instead
>> of allocating it.
>>
>>> +
>>> + /* 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;
>>
>> Only in command mode?
>
> Hi Dmitry,
>
> The intention of the patch wasn't to enable DSC for only command mode.
>
> We didn't want to limit DSC to only command mode because, while the MSM
> DPU driver isn't able to validate DSC on video mode, other vendors might
> have already validated DSC on video mode and would benefit from this patch.
>
> FWIW, inital driver commit [1] notes that the panel is meant to work
> with compressed streams in general and DSC support was tob be added
> later on.
Ack.
>
> Thanks,
>
> Jessica Zhang
>
> [1] https://patchwork.freedesktop.org/patch/517483/?series=112369&rev=2
>
>>
>>>
>>> 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;
>>
>> Keep the line split please.
>>
>>> + 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,
>>> --
>>> 2.41.0
>>>
>>
>>
>> --
>> With best wishes
>> Dmitry
--
With best wishes
Dmitry
More information about the Freedreno
mailing list