Re: [PATCH v3 11/23] drm/panel: Add support for S6E3HA8 panel driver

2024-06-20 Thread Dmitry Baryshkov
On Wed, Jun 19, 2024 at 04:27:40PM GMT, Dzmitry Sankouski wrote:
> вт, 18 июн. 2024 г. в 21:39, Dmitry Baryshkov :
> >
> > > + ret = mipi_dsi_compression_mode(dsi, true);
> > > + if (ret < 0) {
> > > + dev_err(dev, "Failed to set compression mode: %d\n", ret);
> > > + return ret;
> > > + }
> >
> > Interesting, compression mode is being set before the PPS programming?
> >
> Yes, as per vendor kernel:
> https://github.com/klabit87/twrp_android_samsung_kernel_sdm845/blob/e8bb63039008e1704a2f1bde68d39ded9c16ea88/drivers/gpu/drm/msm/samsung/S6E3HA8_AMB577PX01/dsi_panel_S6E3HA8_AMB577PX01_wqhd_octa_cmd.dtsi#L5508

Ack

-- 
With best wishes
Dmitry


Re: [PATCH v3 11/23] drm/panel: Add support for S6E3HA8 panel driver

2024-06-19 Thread Dzmitry Sankouski
вт, 18 июн. 2024 г. в 21:39, Dmitry Baryshkov :
>
> > + ret = mipi_dsi_compression_mode(dsi, true);
> > + if (ret < 0) {
> > + dev_err(dev, "Failed to set compression mode: %d\n", ret);
> > + return ret;
> > + }
>
> Interesting, compression mode is being set before the PPS programming?
>
Yes, as per vendor kernel:
https://github.com/klabit87/twrp_android_samsung_kernel_sdm845/blob/e8bb63039008e1704a2f1bde68d39ded9c16ea88/drivers/gpu/drm/msm/samsung/S6E3HA8_AMB577PX01/dsi_panel_S6E3HA8_AMB577PX01_wqhd_octa_cmd.dtsi#L5508


Re: [PATCH v3 11/23] drm/panel: Add support for S6E3HA8 panel driver

2024-06-18 Thread Dmitry Baryshkov
On Tue, Jun 18, 2024 at 04:59:45PM GMT, Dzmitry Sankouski wrote:
> Add support for MIPI-DSI based S6E3HA8 AMOLED panel
> driver. This panel has 1440x2960 resolution, 5.8-inch physical
> size, and can be found in starqltechn device.
> Brightness regulation is not yet supported.
> 
> Signed-off-by: Dzmitry Sankouski 
> ---
>  MAINTAINERS   |   1 +
>  drivers/gpu/drm/panel/Kconfig |   7 +
>  drivers/gpu/drm/panel/Makefile|   1 +
>  drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c | 426 
> ++
>  4 files changed, 435 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 92a5d0a56353..fae3b8ea9ce4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7154,6 +7154,7 @@ DRM DRIVER FOR SAMSUNG S6E3HA8 PANELS
>  M:   Dzmitry Sankouski 
>  S:   Maintained
>  F:   Documentation/devicetree/bindings/display/panel/samsung,s6e3ha8.yaml
> +F:   drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c
>  
>  DRM DRIVER FOR SITRONIX ST7586 PANELS
>  M:   David Lechner 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 2ae0eb0638f3..903fc8c155c0 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -651,6 +651,13 @@ config DRM_PANEL_SAMSUNG_S6E3HA2
>   depends on BACKLIGHT_CLASS_DEVICE
>   select VIDEOMODE_HELPERS
>  
> +config DRM_PANEL_SAMSUNG_S6E3HA8
> + tristate "Samsung S6E3HA8 DSI video mode panel"
> + depends on OF
> + depends on DRM_MIPI_DSI
> + depends on BACKLIGHT_CLASS_DEVICE
> + select VIDEOMODE_HELPERS
> +
>  config DRM_PANEL_SAMSUNG_S6E63J0X03
>   tristate "Samsung S6E63J0X03 DSI command mode panel"
>   depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index f0203f6e02f4..71774cc5820a 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -66,6 +66,7 @@ obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D27A1) += 
> panel-samsung-s6d27a1.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D7AA0) += panel-samsung-s6d7aa0.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3FA7) += panel-samsung-s6e3fa7.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
> +obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA8) += panel-samsung-s6e3ha8.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63J0X03) += panel-samsung-s6e63j0x03.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63M0) += panel-samsung-s6e63m0.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63M0_SPI) += panel-samsung-s6e63m0-spi.o
> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c 
> b/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c
> new file mode 100644
> index ..49d629643171
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c
> @@ -0,0 +1,426 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Generated with linux-mdss-dsi-panel-driver-generator from vendor device 
> tree:
> + *  Copyright (c) 2013, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2024 Dzmitry Sankouski 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct s6e3ha8 {
> + struct drm_panel panel;
> + struct mipi_dsi_device *dsi;
> + struct drm_dsc_config dsc;
> + struct gpio_desc *reset_gpio;
> + struct regulator_bulk_data supplies[3];
> +};
> +
> +static inline
> +struct s6e3ha8 *to_s6e3ha8_amb577px01_wqhd(struct drm_panel *panel)
> +{
> + return container_of(panel, struct s6e3ha8, panel);
> +}
> +
> +#define s6e3ha8_call_write_func(ret, func) do {  \
> + ret = (func);   \
> + if (ret < 0)\
> + return ret; \
> +} while (0)

Please rework the driver to use mipi_dsi_*_multi() family of functions.
Using the mipi_dsi_multi_context should make this wrapper obsolete too.

> +
> +static int s6e3ha8_test_key_on_lvl1(struct mipi_dsi_device *dsi)
> +{
> + static const u8 d[] = { 0x9f, 0xa5, 0xa5 };
> +
> + return mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d));
> + return 0;

Ugh. So which return is it?

> +}
> +

[...]
> +static int s6e3ha8_power_on(struct s6e3ha8 *ctx)
> +{
> + int ret;
> +
> + ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);

Inline this function, it's just regulator_bulk_enable() in the end.

> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int s6e3ha8_power_off(struct s6e3ha8 *ctx)
> +{
> + return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +}
> +
> +static void s6e3ha8_amb577px01_wqhd_reset(struct s6e3ha8 *ctx)
> +{
> + gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> + usleep_range(5000, 6000);
> + gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> + usleep_range(5000, 6000);
> + gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> + usleep_range(5000, 6000);
> +}
> +
> +stati