2023-11-24 10:45:40

by Shengyang Chen

[permalink] [raw]
Subject: [PATCH v1 0/2] Add waveshare 7inch touchscreen panel support

This patchset adds waveshare 7inch touchscreen panel support
for the StarFive JH7110 SoC.

Patch 1 add new compatible for the raspberrypi panel driver and its dt-binding.
Patch 2 add new display mode and new probing process for raspberrypi panel driver.

Waveshare 7inch touchscreen panel is a kind of raspberrypi panel
which can be drived by raspberrypi panel driver.

The series has been tested on the VisionFive 2 board.

Shengyang Chen (2):
dt-bindings: display: panel: raspberrypi: Add compatible property for
waveshare 7inch touchscreen panel
gpu: drm: panel: raspberrypi: add new display mode and new probing
process

.../panel/raspberrypi,7inch-touchscreen.yaml | 4 +-
.../drm/panel/panel-raspberrypi-touchscreen.c | 99 ++++++++++++++++---
2 files changed, 91 insertions(+), 12 deletions(-)

--
2.17.1


2023-11-24 10:45:40

by Shengyang Chen

[permalink] [raw]
Subject: [PATCH v1 1/2] dt-bindings: display: panel: raspberrypi: Add compatible property for waveshare 7inch touchscreen panel

The waveshare 7inch touchscreen panel is a kind of raspberrypi pi
panel and it can be drived by panel-raspberrypi-touchscreen.c.
Add compatible property for it.

Signed-off-by: Keith Zhao <[email protected]>
Signed-off-by: Shengyang Chen <[email protected]>
---
.../bindings/display/panel/raspberrypi,7inch-touchscreen.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
index 22a083f7bc8e..e4e6cb4d4e5b 100644
--- a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
+++ b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
@@ -22,7 +22,9 @@ description: |+

properties:
compatible:
- const: raspberrypi,7inch-touchscreen-panel
+ enum:
+ - raspberrypi,7inch-touchscreen-panel
+ - waveshare,7inch-touchscreen-panel

reg:
const: 0x45
--
2.17.1

2023-11-24 10:47:53

by Shengyang Chen

[permalink] [raw]
Subject: [PATCH v1 2/2] gpu: drm: panel: raspberrypi: add new display mode and new probing process

The waveshare 7inch touchscreen panel is a kind of raspberrypi
pi panel and it can be drived by panel-raspberrypi-touchscreen.c.
Add new display mode for it.

In order to fit CDNS DSI driver which used by StarFive SoCs like JH7110,
add new probing process for it. The compatible is used to distinguishing.

Signed-off-by: Keith Zhao <[email protected]>
Signed-off-by: Shengyang Chen <[email protected]>
---
.../drm/panel/panel-raspberrypi-touchscreen.c | 99 ++++++++++++++++---
1 file changed, 88 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
index 4618c892cdd6..4478f1568205 100644
--- a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
+++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
@@ -194,6 +194,13 @@ struct rpi_touchscreen {
struct i2c_client *i2c;
};

+struct touchscreen_info {
+ const struct drm_display_mode *display_mode;
+ u32 display_mode_size;
+ int (*touchscreen_post_probe)(struct rpi_touchscreen *ts, struct mipi_dsi_host *host,
+ struct mipi_dsi_device_info *info);
+};
+
static const struct drm_display_mode rpi_touchscreen_modes[] = {
{
/* Modeline comes from the Raspberry Pi firmware, with HFP=1
@@ -211,6 +218,20 @@ static const struct drm_display_mode rpi_touchscreen_modes[] = {
},
};

+static const struct drm_display_mode ws_touchscreen_modes[] = {
+ {
+ .clock = 29700000 / 1000,
+ .hdisplay = 800,
+ .hsync_start = 800 + 90,
+ .hsync_end = 800 + 90 + 5,
+ .htotal = 800 + 90 + 5 + 5,
+ .vdisplay = 480,
+ .vsync_start = 480 + 60,
+ .vsync_end = 480 + 60 + 5,
+ .vtotal = 480 + 60 + 5 + 5,
+ },
+};
+
static struct rpi_touchscreen *panel_to_ts(struct drm_panel *panel)
{
return container_of(panel, struct rpi_touchscreen, base);
@@ -319,9 +340,13 @@ static int rpi_touchscreen_get_modes(struct drm_panel *panel,
{
unsigned int i, num = 0;
static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+ const struct touchscreen_info *screen_info;
+
+ screen_info = of_device_get_match_data(panel->dev);
+
+ for (i = 0; i < screen_info->display_mode_size; i++) {
+ const struct drm_display_mode *m = &screen_info->display_mode[i];

- for (i = 0; i < ARRAY_SIZE(rpi_touchscreen_modes); i++) {
- const struct drm_display_mode *m = &rpi_touchscreen_modes[i];
struct drm_display_mode *mode;

mode = drm_mode_duplicate(connector->dev, m);
@@ -360,12 +385,13 @@ static const struct drm_panel_funcs rpi_touchscreen_funcs = {
.get_modes = rpi_touchscreen_get_modes,
};

-static int rpi_touchscreen_probe(struct i2c_client *i2c)
+static int touchscreen_probe(struct i2c_client *i2c)
{
struct device *dev = &i2c->dev;
struct rpi_touchscreen *ts;
struct device_node *endpoint, *dsi_host_node;
struct mipi_dsi_host *host;
+ const struct touchscreen_info *screen_info;
int ver;
struct mipi_dsi_device_info info = {
.type = RPI_DSI_DRIVER_NAME,
@@ -421,14 +447,29 @@ static int rpi_touchscreen_probe(struct i2c_client *i2c)

of_node_put(endpoint);

- ts->dsi = mipi_dsi_device_register_full(host, &info);
+ screen_info = of_device_get_match_data(&i2c->dev);
+ if (!screen_info->touchscreen_post_probe)
+ return -ENODEV;
+
+ return screen_info->touchscreen_post_probe(ts, host, &info);
+
+error:
+ of_node_put(endpoint);
+
+ return -ENODEV;
+}
+
+static int rpi_touchscreen_probe(struct rpi_touchscreen *ts, struct mipi_dsi_host *host,
+ struct mipi_dsi_device_info *info)
+{
+ ts->dsi = mipi_dsi_device_register_full(host, info);
if (IS_ERR(ts->dsi)) {
- dev_err(dev, "DSI device registration failed: %ld\n",
+ dev_err(&ts->i2c->dev, "DSI device registration failed: %ld\n",
PTR_ERR(ts->dsi));
return PTR_ERR(ts->dsi);
}

- drm_panel_init(&ts->base, dev, &rpi_touchscreen_funcs,
+ drm_panel_init(&ts->base, &ts->i2c->dev, &rpi_touchscreen_funcs,
DRM_MODE_CONNECTOR_DSI);

/* This appears last, as it's what will unblock the DSI host
@@ -437,10 +478,33 @@ static int rpi_touchscreen_probe(struct i2c_client *i2c)
drm_panel_add(&ts->base);

return 0;
+}

-error:
- of_node_put(endpoint);
- return -ENODEV;
+static int ws_touchscreen_probe(struct rpi_touchscreen *ts, struct mipi_dsi_host *host,
+ struct mipi_dsi_device_info *info)
+
+{
+ /* in order to match CDNS DSI driver , it is nessary
+ * to ensure drm_panel_init() & drm_panel_add() before
+ * mipi_dsi_device_register_full()
+ *
+ * after mipi_dsi_device_register_full finished , it will
+ * run rpi_touchscreen_dsi_probe -> mipi_dsi_attach()
+ * the CDNS DSI attach helper fun need to find the panel by
+ * of_drm_find_panel( ). so need add panel before the register.
+ */
+ drm_panel_init(&ts->base, &ts->i2c->dev, &rpi_touchscreen_funcs,
+ DRM_MODE_CONNECTOR_DSI);
+ drm_panel_add(&ts->base);
+
+ ts->dsi = mipi_dsi_device_register_full(host, info);
+ if (IS_ERR(ts->dsi)) {
+ dev_err(&ts->i2c->dev, "DSI device registration failed: %ld\n",
+ PTR_ERR(ts->dsi));
+ return PTR_ERR(ts->dsi);
+ }
+
+ return 0;
}

static void rpi_touchscreen_remove(struct i2c_client *i2c)
@@ -477,8 +541,21 @@ static struct mipi_dsi_driver rpi_touchscreen_dsi_driver = {
.probe = rpi_touchscreen_dsi_probe,
};

+static const struct touchscreen_info rpi_touchscreen_info = {
+ .display_mode = rpi_touchscreen_modes,
+ .display_mode_size = ARRAY_SIZE(rpi_touchscreen_modes),
+ .touchscreen_post_probe = rpi_touchscreen_probe,
+};
+
+static const struct touchscreen_info ws_touchscreen_info = {
+ .display_mode = ws_touchscreen_modes,
+ .display_mode_size = ARRAY_SIZE(ws_touchscreen_modes),
+ .touchscreen_post_probe = ws_touchscreen_probe,
+};
+
static const struct of_device_id rpi_touchscreen_of_ids[] = {
- { .compatible = "raspberrypi,7inch-touchscreen-panel" },
+ { .compatible = "raspberrypi,7inch-touchscreen-panel", .data = &rpi_touchscreen_info,},
+ { .compatible = "waveshare,7inch-touchscreen-panel", .data = &ws_touchscreen_info,},
{ } /* sentinel */
};
MODULE_DEVICE_TABLE(of, rpi_touchscreen_of_ids);
@@ -488,7 +565,7 @@ static struct i2c_driver rpi_touchscreen_driver = {
.name = "rpi_touchscreen",
.of_match_table = rpi_touchscreen_of_ids,
},
- .probe = rpi_touchscreen_probe,
+ .probe = touchscreen_probe,
.remove = rpi_touchscreen_remove,
};

--
2.17.1

2023-11-24 12:31:28

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] dt-bindings: display: panel: raspberrypi: Add compatible property for waveshare 7inch touchscreen panel

On Fri, Nov 24, 2023 at 06:44:50PM +0800, Shengyang Chen wrote:
> The waveshare 7inch touchscreen panel is a kind of raspberrypi pi
> panel

Can you be more specific about what "is a kind of rpi panel" means?
Are they using identical chips as controllers or something like that?

> and it can be drived by panel-raspberrypi-touchscreen.c.
> Add compatible property for it.
>
> Signed-off-by: Keith Zhao <[email protected]>
> Signed-off-by: Shengyang Chen <[email protected]>
> ---
> .../bindings/display/panel/raspberrypi,7inch-touchscreen.yaml | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
> index 22a083f7bc8e..e4e6cb4d4e5b 100644
> --- a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
> @@ -22,7 +22,9 @@ description: |+
>
> properties:
> compatible:
> - const: raspberrypi,7inch-touchscreen-panel
> + enum:
> + - raspberrypi,7inch-touchscreen-panel
> + - waveshare,7inch-touchscreen-panel
>
> reg:
> const: 0x45
> --
> 2.17.1
>


Attachments:
(No filename) (1.32 kB)
signature.asc (235.00 B)
Download all attachments

2023-11-24 16:05:23

by Dave Stevenson

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Add waveshare 7inch touchscreen panel support

On Fri, 24 Nov 2023 at 15:00, Stefan Wahren <[email protected]> wrote:
>
> Hi Shengyang,
>
> [fix address of Emma]

Not merged to master yet, but Emma has stepped back from maintenance.
https://lists.freedesktop.org/archives/dri-devel/2023-October/428829.html
Dropped from the cc.

> Am 24.11.23 um 11:44 schrieb Shengyang Chen:
> > This patchset adds waveshare 7inch touchscreen panel support
> > for the StarFive JH7110 SoC.
> >
> > Patch 1 add new compatible for the raspberrypi panel driver and its dt-binding.
> > Patch 2 add new display mode and new probing process for raspberrypi panel driver.
> >
> > Waveshare 7inch touchscreen panel is a kind of raspberrypi panel
> > which can be drived by raspberrypi panel driver.
> >
> > The series has been tested on the VisionFive 2 board.
> surprisingly i was recently working on the official Raspberry Pi
> touchscreen and was able to get it running the new way.
>
> What do i mean with the new way. There is almost nothing special to the
> Raspberry Pi touchscreen, so we should try to use/extend existing
> components like:
>
> CONFIG_DRM_PANEL_SIMPLE
> CONFIG_TOUCHSCREEN_EDT_FT5X06
> CONFIG_DRM_TOSHIBA_TC358762
>
> The only special part is the Attiny on the connector PCB which requires:
>
> CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_ATTINY
>
> So the whole point is to avoid writing monolitic drivers for simple
> panel like that.
>
> There is a WIP branch based on top of Linux 6.7-rcX, which should
> demonstrate this approach [1]. Unfortunately it is not ready for
> upstreaming, but it has been tested on a Raspberry Pi 3 B Plus. Maybe
> this is helpful for your case.
>
> Actually i consider panel-raspberrypi-touchscreen.c as a dead end, which
> shouldn't be extended.

Agreed.

The panel control being bound in with the Atmel control has no hook
for the EDT5x06 touch driver to hook in and keep the power to the
touch controller active. When the panel disable gets called, bye bye
touch overlay :-(

And I'm reading the driver change as more of a hack to get it to work
on your platform, not as adding support for the Waveshare panel
variant.
Waveshare deliberately cloned the behaviour of the Pi 7" panel in
order to make it work with the old Pi firmware drivers, so it
shouldn't need any significant changes. Where did the new timings come
from?

Dave

> Btw there are already DT overlays in mainline which seems to use the
> Raspberry Pi 7inch panel (without touch function yet) [2].
>
> [1] - https://github.com/lategoodbye/rpi-zero/commits/v6.7-7inch-ts
> [2] -
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x-rpidsi.dtso?h=v6.6.2&id=6b4da1354fd81adace0cda448c77d8f2a47d8474
>
> >
> > Shengyang Chen (2):
> > dt-bindings: display: panel: raspberrypi: Add compatible property for
> > waveshare 7inch touchscreen panel
> > gpu: drm: panel: raspberrypi: add new display mode and new probing
> > process
> >
> > .../panel/raspberrypi,7inch-touchscreen.yaml | 4 +-
> > .../drm/panel/panel-raspberrypi-touchscreen.c | 99 ++++++++++++++++---
> > 2 files changed, 91 insertions(+), 12 deletions(-)
> >
>

2023-12-06 08:56:26

by Keith Zhao

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Add waveshare 7inch touchscreen panel support



On 2023/11/25 0:04, Dave Stevenson wrote:
> On Fri, 24 Nov 2023 at 15:00, Stefan Wahren <[email protected]> wrote:
>>
>> Hi Shengyang,
>>
>> [fix address of Emma]
>
> Not merged to master yet, but Emma has stepped back from maintenance.
> https://lists.freedesktop.org/archives/dri-devel/2023-October/428829.html
> Dropped from the cc.
>
>> Am 24.11.23 um 11:44 schrieb Shengyang Chen:
>> > This patchset adds waveshare 7inch touchscreen panel support
>> > for the StarFive JH7110 SoC.
>> >
>> > Patch 1 add new compatible for the raspberrypi panel driver and its dt-binding.
>> > Patch 2 add new display mode and new probing process for raspberrypi panel driver.
>> >
>> > Waveshare 7inch touchscreen panel is a kind of raspberrypi panel
>> > which can be drived by raspberrypi panel driver.
>> >
>> > The series has been tested on the VisionFive 2 board.
>> surprisingly i was recently working on the official Raspberry Pi
>> touchscreen and was able to get it running the new way.
>>
>> What do i mean with the new way. There is almost nothing special to the
>> Raspberry Pi touchscreen, so we should try to use/extend existing
>> components like:
>>
>> CONFIG_DRM_PANEL_SIMPLE
>> CONFIG_TOUCHSCREEN_EDT_FT5X06
>> CONFIG_DRM_TOSHIBA_TC358762
>>
>> The only special part is the Attiny on the connector PCB which requires:
>>
>> CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_ATTINY
>>
>> So the whole point is to avoid writing monolitic drivers for simple
>> panel like that.
>>
>> There is a WIP branch based on top of Linux 6.7-rcX, which should
>> demonstrate this approach [1]. Unfortunately it is not ready for
>> upstreaming, but it has been tested on a Raspberry Pi 3 B Plus. Maybe
>> this is helpful for your case.
>>
>> Actually i consider panel-raspberrypi-touchscreen.c as a dead end, which
>> shouldn't be extended.
>
> Agreed.
>
> The panel control being bound in with the Atmel control has no hook
> for the EDT5x06 touch driver to hook in and keep the power to the
> touch controller active. When the panel disable gets called, bye bye
> touch overlay :-(
>
> And I'm reading the driver change as more of a hack to get it to work
> on your platform, not as adding support for the Waveshare panel
> variant.
> Waveshare deliberately cloned the behaviour of the Pi 7" panel in
> order to make it work with the old Pi firmware drivers, so it
> shouldn't need any significant changes. Where did the new timings come
> from?
>
> Dave
hi Dave :
that's means the panel driver split into 3 sub-modules:
panel + panel_bridge + regulator.

I have a question: in the
static int rpi_touchscreen_probe(struct i2c_client *i2c)
{
......

ver = rpi_touchscreen_i2c_read(ts, REG_ID);
if (ver < 0) {
dev_err(dev, "Atmel I2C read failed: %d\n", ver);
return -ENODEV;
}

switch (ver) {
case 0xde: /* ver 1 */
case 0xc3: /* ver 2 */
break;
default:
dev_err(dev, "Unknown Atmel firmware revision: 0x%02x\n", ver);
return -ENODEV;
}

......
}
i think this "I2C read" can use to detect whether the panel is connected to dsi controller.

and when split the panel driver into 3 sub-modules, it seems the default way is connected.
if I drop the panel , run modetest to check the connector status , result connected.
Is there any way to detect the connection in this case?
Thanks



-------------------------------------

Where did the new timings come from?

-------------------------------------
My platform dphy tx hardware has certain limitations.
Only supports integer multiples of 10M bitrate:
such as 160M ,170M, 180M,190M,...1G(max)

as common dphy bitrate = pixclock*bpp/lanes.
This value cannot match successfully in most cases.

so in order to match bitrate , I choose a bitrate value around pixclock*bpp/lanes,
Prevent overflow and underflow by fine-tuning the timing parameters:-(
that will make the new timming value.

>
>> Btw there are already DT overlays in mainline which seems to use the
>> Raspberry Pi 7inch panel (without touch function yet) [2].
>>
>> [1] - https://github.com/lategoodbye/rpi-zero/commits/v6.7-7inch-ts
>> [2] -
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x-rpidsi.dtso?h=v6.6.2&id=6b4da1354fd81adace0cda448c77d8f2a47d8474
>>
>> >
>> > Shengyang Chen (2):
>> > dt-bindings: display: panel: raspberrypi: Add compatible property for
>> > waveshare 7inch touchscreen panel
>> > gpu: drm: panel: raspberrypi: add new display mode and new probing
>> > process
>> >
>> > .../panel/raspberrypi,7inch-touchscreen.yaml | 4 +-
>> > .../drm/panel/panel-raspberrypi-touchscreen.c | 99 ++++++++++++++++---
>> > 2 files changed, 91 insertions(+), 12 deletions(-)
>> >
>>

2023-12-06 09:47:52

by Shengyang Chen

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] dt-bindings: display: panel: raspberrypi: Add compatible property for waveshare 7inch touchscreen panel

Hi, Conor

On 2023/11/24 20:31, Conor Dooley wrote:
> On Fri, Nov 24, 2023 at 06:44:50PM +0800, Shengyang Chen wrote:
>> The waveshare 7inch touchscreen panel is a kind of raspberrypi pi
>> panel
>
> Can you be more specific about what "is a kind of rpi panel" means?
> Are they using identical chips as controllers or something like that?
>

Wareshare panel has same i2c slave address and registers address with
the original raspberry pi panel. They both use Atmel firmware and they
got same reg id. It can be operated by using the driver of raspberry pi driver
after some change of the code. So I suppose it may be a kind of raspberry pi panel
and discribe it in this way. It's my own judgement. Sorry about that.
Maybe just like Dave said, It cloned the behaviour of the raspberri pi panel.
I will change the discribtion in next version to not make other confused.

By the way, we will try Stefan's method before next version.
The method we used in this patch may be abandoned if Stefan's method is verified in our platform.
At that time yaml may also be changed to fit new method.


>> and it can be drived by panel-raspberrypi-touchscreen.c.
>> Add compatible property for it.
>>
>> Signed-off-by: Keith Zhao <[email protected]>
>> Signed-off-by: Shengyang Chen <[email protected]>
>> ---
>> .../bindings/display/panel/raspberrypi,7inch-touchscreen.yaml | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
>> index 22a083f7bc8e..e4e6cb4d4e5b 100644
>> --- a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
>> +++ b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
>> @@ -22,7 +22,9 @@ description: |+
>>
>> properties:
>> compatible:
>> - const: raspberrypi,7inch-touchscreen-panel
>> + enum:
>> + - raspberrypi,7inch-touchscreen-panel
>> + - waveshare,7inch-touchscreen-panel
>>
>> reg:
>> const: 0x45
>> --
>> 2.17.1
>>


thanks.

Best Regards,
Shengyang

2023-12-06 09:57:00

by Shengyang Chen

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Add waveshare 7inch touchscreen panel support

Hi, Stefan

Thanks for your comment and review

On 2023/11/24 22:54, Stefan Wahren wrote:
> Hi Shengyang,
>
> [fix address of Emma]
>
> Am 24.11.23 um 11:44 schrieb Shengyang Chen:
>> This patchset adds waveshare 7inch touchscreen panel support
>> for the StarFive JH7110 SoC.
>>
>> Patch 1 add new compatible for the raspberrypi panel driver and its dt-binding.
>> Patch 2 add new display mode and new probing process for raspberrypi panel driver.
>>
>> Waveshare 7inch touchscreen panel is a kind of raspberrypi panel
>> which can be drived by raspberrypi panel driver.
>>
>> The series has been tested on the VisionFive 2 board.
> surprisingly i was recently working on the official Raspberry Pi
> touchscreen and was able to get it running the new way.
>
> What do i mean with the new way. There is almost nothing special to the
> Raspberry Pi touchscreen, so we should try to use/extend existing
> components like:
>
> CONFIG_DRM_PANEL_SIMPLE
> CONFIG_TOUCHSCREEN_EDT_FT5X06
> CONFIG_DRM_TOSHIBA_TC358762
>
> The only special part is the Attiny on the connector PCB which requires:
>
> CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_ATTINY
>
> So the whole point is to avoid writing monolitic drivers for simple
> panel like that.
>
> There is a WIP branch based on top of Linux 6.7-rcX, which should
> demonstrate this approach [1]. Unfortunately it is not ready for
> upstreaming, but it has been tested on a Raspberry Pi 3 B Plus. Maybe
> this is helpful for your case.
>
> Actually i consider panel-raspberrypi-touchscreen.c as a dead end, which
> shouldn't be extended.
>
> Btw there are already DT overlays in mainline which seems to use the
> Raspberry Pi 7inch panel (without touch function yet) [2].
>
> [1] - https://github.com/lategoodbye/rpi-zero/commits/v6.7-7inch-ts
> [2] -
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x-rpidsi.dtso?h=v6.6.2&id=6b4da1354fd81adace0cda448c77d8f2a47d8474
>

Thank you very much for your advice. We will try this method before making new patch.
This method will be used if its verified in our soc.
If there is any problem, we may continue to contact. Thanks a lot.

>>
>> Shengyang Chen (2):
>>    dt-bindings: display: panel: raspberrypi: Add compatible property for
>>      waveshare 7inch touchscreen panel
>>    gpu: drm: panel: raspberrypi: add new display mode and new probing
>>      process
>>
>>   .../panel/raspberrypi,7inch-touchscreen.yaml  |  4 +-
>>   .../drm/panel/panel-raspberrypi-touchscreen.c | 99 ++++++++++++++++---
>>   2 files changed, 91 insertions(+), 12 deletions(-)
>>
>


thanks

Best Regards,
Shengyang

2023-12-06 14:57:26

by Dave Stevenson

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Add waveshare 7inch touchscreen panel support

Hi Keith

On Wed, 6 Dec 2023 at 08:55, Keith Zhao <[email protected]> wrote:
>
>
>
> On 2023/11/25 0:04, Dave Stevenson wrote:
> > On Fri, 24 Nov 2023 at 15:00, Stefan Wahren <[email protected]> wrote:
> >>
> >> Hi Shengyang,
> >>
> >> [fix address of Emma]
> >
> > Not merged to master yet, but Emma has stepped back from maintenance.
> > https://lists.freedesktop.org/archives/dri-devel/2023-October/428829.html
> > Dropped from the cc.
> >
> >> Am 24.11.23 um 11:44 schrieb Shengyang Chen:
> >> > This patchset adds waveshare 7inch touchscreen panel support
> >> > for the StarFive JH7110 SoC.
> >> >
> >> > Patch 1 add new compatible for the raspberrypi panel driver and its dt-binding.
> >> > Patch 2 add new display mode and new probing process for raspberrypi panel driver.
> >> >
> >> > Waveshare 7inch touchscreen panel is a kind of raspberrypi panel
> >> > which can be drived by raspberrypi panel driver.
> >> >
> >> > The series has been tested on the VisionFive 2 board.
> >> surprisingly i was recently working on the official Raspberry Pi
> >> touchscreen and was able to get it running the new way.
> >>
> >> What do i mean with the new way. There is almost nothing special to the
> >> Raspberry Pi touchscreen, so we should try to use/extend existing
> >> components like:
> >>
> >> CONFIG_DRM_PANEL_SIMPLE
> >> CONFIG_TOUCHSCREEN_EDT_FT5X06
> >> CONFIG_DRM_TOSHIBA_TC358762
> >>
> >> The only special part is the Attiny on the connector PCB which requires:
> >>
> >> CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_ATTINY
> >>
> >> So the whole point is to avoid writing monolitic drivers for simple
> >> panel like that.
> >>
> >> There is a WIP branch based on top of Linux 6.7-rcX, which should
> >> demonstrate this approach [1]. Unfortunately it is not ready for
> >> upstreaming, but it has been tested on a Raspberry Pi 3 B Plus. Maybe
> >> this is helpful for your case.
> >>
> >> Actually i consider panel-raspberrypi-touchscreen.c as a dead end, which
> >> shouldn't be extended.
> >
> > Agreed.
> >
> > The panel control being bound in with the Atmel control has no hook
> > for the EDT5x06 touch driver to hook in and keep the power to the
> > touch controller active. When the panel disable gets called, bye bye
> > touch overlay :-(
> >
> > And I'm reading the driver change as more of a hack to get it to work
> > on your platform, not as adding support for the Waveshare panel
> > variant.
> > Waveshare deliberately cloned the behaviour of the Pi 7" panel in
> > order to make it work with the old Pi firmware drivers, so it
> > shouldn't need any significant changes. Where did the new timings come
> > from?
> >
> > Dave
> hi Dave :
> that's means the panel driver split into 3 sub-modules:
> panel + panel_bridge + regulator.

Correct.

You'll have a fourth device in edt_ft5x06 for the touch overlay too,
which will link to the regulator driver for power control.

> I have a question: in the
> static int rpi_touchscreen_probe(struct i2c_client *i2c)
> {
> ......
>
> ver = rpi_touchscreen_i2c_read(ts, REG_ID);
> if (ver < 0) {
> dev_err(dev, "Atmel I2C read failed: %d\n", ver);
> return -ENODEV;
> }
>
> switch (ver) {
> case 0xde: /* ver 1 */
> case 0xc3: /* ver 2 */
> break;
> default:
> dev_err(dev, "Unknown Atmel firmware revision: 0x%02x\n", ver);
> return -ENODEV;
> }
>
> ......
> }
> i think this "I2C read" can use to detect whether the panel is connected to dsi controller.
>
> and when split the panel driver into 3 sub-modules, it seems the default way is connected.
> if I drop the panel , run modetest to check the connector status , result connected.
> Is there any way to detect the connection in this case?
> Thanks

I am not aware of any DSI drivers that support hotplugging, therefore
the connector state will always be connected if the device probes.

On vc4 the relevant DSI host controller has to have been enabled in
device tree and will be a required component for binding. The DSI host
controller will be waiting on the DSI peripheral driver to call
mipi_dsi_attach, which then calls component_add. If the panel or panel
regulator isn't present, then that never happens if the panel isn't
present, so vc4 won't bind.
It is a little ugly in that you lose the whole DRM card, but that is
how I understand DRM is generally set up to work for DSI or similar
display interfaces.

> -------------------------------------
>
> Where did the new timings come from?
>
> -------------------------------------
> My platform dphy tx hardware has certain limitations.
> Only supports integer multiples of 10M bitrate:
> such as 160M ,170M, 180M,190M,...1G(max)
>
> as common dphy bitrate = pixclock*bpp/lanes.
> This value cannot match successfully in most cases.
>
> so in order to match bitrate , I choose a bitrate value around pixclock*bpp/lanes,
> Prevent overflow and underflow by fine-tuning the timing parameters:-(
> that will make the new timming value.

That isn't really a function of the panel then.

All DRM bridges have the option to define a mode_fixup in
drm_bridge_funcs, and that gives you the option to adjust the timings
as required.

vc4 has a similar constraint in that the PHY only has an integer
divider from a 2 or 3GHz PLL. It implements mode_fixup to compute the
next highest pixel clock, and then adjusts the horizontal front porch
to keep the same line timing. See
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/vc4/vc4_dsi.c#L825

Dave

> >
> >> Btw there are already DT overlays in mainline which seems to use the
> >> Raspberry Pi 7inch panel (without touch function yet) [2].
> >>
> >> [1] - https://github.com/lategoodbye/rpi-zero/commits/v6.7-7inch-ts
> >> [2] -
> >> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x-rpidsi.dtso?h=v6.6.2&id=6b4da1354fd81adace0cda448c77d8f2a47d8474
> >>
> >> >
> >> > Shengyang Chen (2):
> >> > dt-bindings: display: panel: raspberrypi: Add compatible property for
> >> > waveshare 7inch touchscreen panel
> >> > gpu: drm: panel: raspberrypi: add new display mode and new probing
> >> > process
> >> >
> >> > .../panel/raspberrypi,7inch-touchscreen.yaml | 4 +-
> >> > .../drm/panel/panel-raspberrypi-touchscreen.c | 99 ++++++++++++++++---
> >> > 2 files changed, 91 insertions(+), 12 deletions(-)
> >> >
> >>

2023-12-06 15:42:01

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] dt-bindings: display: panel: raspberrypi: Add compatible property for waveshare 7inch touchscreen panel

On Wed, Dec 06, 2023 at 05:43:48PM +0800, Shengyang Chen wrote:
> Hi, Conor
>
> On 2023/11/24 20:31, Conor Dooley wrote:
> > On Fri, Nov 24, 2023 at 06:44:50PM +0800, Shengyang Chen wrote:
> >> The waveshare 7inch touchscreen panel is a kind of raspberrypi pi
> >> panel
> >
> > Can you be more specific about what "is a kind of rpi panel" means?
> > Are they using identical chips as controllers or something like that?
> >
>
> Wareshare panel has same i2c slave address and registers address with
> the original raspberry pi panel. They both use Atmel firmware and they
> got same reg id. It can be operated by using the driver of raspberry pi driver
> after some change of the code. So I suppose it may be a kind of raspberry pi panel
> and discribe it in this way. It's my own judgement. Sorry about that.
> Maybe just like Dave said, It cloned the behaviour of the raspberri pi panel.
> I will change the discribtion in next version to not make other confused.
>
> By the way, we will try Stefan's method before next version.
> The method we used in this patch may be abandoned if Stefan's method is verified in our platform.
> At that time yaml may also be changed to fit new method.

I don't know what Stefan's approach is, but I do not think that a
bindings patch should be dropped. The waveshare might be a clone, but it
is a distinct device. If the same driver can control both, then the
compatible setups that should be permitted are:
compatible = "raspberrypi,7inch-touchscreen-panel";
and
compatible = "waveshare,7inch-touchscreen-panel", "raspberrypi,7inch-touchscreen-panel";

Cheers,
Conor.

> >> and it can be drived by panel-raspberrypi-touchscreen.c.
> >> Add compatible property for it.
> >>
> >> Signed-off-by: Keith Zhao <[email protected]>
> >> Signed-off-by: Shengyang Chen <[email protected]>
> >> ---
> >> .../bindings/display/panel/raspberrypi,7inch-touchscreen.yaml | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
> >> index 22a083f7bc8e..e4e6cb4d4e5b 100644
> >> --- a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
> >> +++ b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
> >> @@ -22,7 +22,9 @@ description: |+
> >>
> >> properties:
> >> compatible:
> >> - const: raspberrypi,7inch-touchscreen-panel
> >> + enum:
> >> + - raspberrypi,7inch-touchscreen-panel
> >> + - waveshare,7inch-touchscreen-panel
> >>
> >> reg:
> >> const: 0x45
> >> --
> >> 2.17.1
> >>
>
>
> thanks.
>
> Best Regards,
> Shengyang
>


Attachments:
(No filename) (2.79 kB)
signature.asc (235.00 B)
Download all attachments

2023-12-07 02:17:31

by Keith Zhao

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Add waveshare 7inch touchscreen panel support



On 2023/12/6 22:56, Dave Stevenson wrote:
> Hi Keith
>
> On Wed, 6 Dec 2023 at 08:55, Keith Zhao <[email protected]> wrote:
>>
>>
>>
>> On 2023/11/25 0:04, Dave Stevenson wrote:
>> > On Fri, 24 Nov 2023 at 15:00, Stefan Wahren <[email protected]> wrote:
>> >>
>> >> Hi Shengyang,
>> >>
>> >> [fix address of Emma]
>> >
>> > Not merged to master yet, but Emma has stepped back from maintenance.
>> > https://lists.freedesktop.org/archives/dri-devel/2023-October/428829.html
>> > Dropped from the cc.
>> >
>> >> Am 24.11.23 um 11:44 schrieb Shengyang Chen:
>> >> > This patchset adds waveshare 7inch touchscreen panel support
>> >> > for the StarFive JH7110 SoC.
>> >> >
>> >> > Patch 1 add new compatible for the raspberrypi panel driver and its dt-binding.
>> >> > Patch 2 add new display mode and new probing process for raspberrypi panel driver.
>> >> >
>> >> > Waveshare 7inch touchscreen panel is a kind of raspberrypi panel
>> >> > which can be drived by raspberrypi panel driver.
>> >> >
>> >> > The series has been tested on the VisionFive 2 board.
>> >> surprisingly i was recently working on the official Raspberry Pi
>> >> touchscreen and was able to get it running the new way.
>> >>
>> >> What do i mean with the new way. There is almost nothing special to the
>> >> Raspberry Pi touchscreen, so we should try to use/extend existing
>> >> components like:
>> >>
>> >> CONFIG_DRM_PANEL_SIMPLE
>> >> CONFIG_TOUCHSCREEN_EDT_FT5X06
>> >> CONFIG_DRM_TOSHIBA_TC358762
>> >>
>> >> The only special part is the Attiny on the connector PCB which requires:
>> >>
>> >> CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_ATTINY
>> >>
>> >> So the whole point is to avoid writing monolitic drivers for simple
>> >> panel like that.
>> >>
>> >> There is a WIP branch based on top of Linux 6.7-rcX, which should
>> >> demonstrate this approach [1]. Unfortunately it is not ready for
>> >> upstreaming, but it has been tested on a Raspberry Pi 3 B Plus. Maybe
>> >> this is helpful for your case.
>> >>
>> >> Actually i consider panel-raspberrypi-touchscreen.c as a dead end, which
>> >> shouldn't be extended.
>> >
>> > Agreed.
>> >
>> > The panel control being bound in with the Atmel control has no hook
>> > for the EDT5x06 touch driver to hook in and keep the power to the
>> > touch controller active. When the panel disable gets called, bye bye
>> > touch overlay :-(
>> >
>> > And I'm reading the driver change as more of a hack to get it to work
>> > on your platform, not as adding support for the Waveshare panel
>> > variant.
>> > Waveshare deliberately cloned the behaviour of the Pi 7" panel in
>> > order to make it work with the old Pi firmware drivers, so it
>> > shouldn't need any significant changes. Where did the new timings come
>> > from?
>> >
>> > Dave
>> hi Dave :
>> that's means the panel driver split into 3 sub-modules:
>> panel + panel_bridge + regulator.
>
> Correct.
>
> You'll have a fourth device in edt_ft5x06 for the touch overlay too,
> which will link to the regulator driver for power control.
>
>> I have a question: in the
>> static int rpi_touchscreen_probe(struct i2c_client *i2c)
>> {
>> ......
>>
>> ver = rpi_touchscreen_i2c_read(ts, REG_ID);
>> if (ver < 0) {
>> dev_err(dev, "Atmel I2C read failed: %d\n", ver);
>> return -ENODEV;
>> }
>>
>> switch (ver) {
>> case 0xde: /* ver 1 */
>> case 0xc3: /* ver 2 */
>> break;
>> default:
>> dev_err(dev, "Unknown Atmel firmware revision: 0x%02x\n", ver);
>> return -ENODEV;
>> }
>>
>> ......
>> }
>> i think this "I2C read" can use to detect whether the panel is connected to dsi controller.
>>
>> and when split the panel driver into 3 sub-modules, it seems the default way is connected.
>> if I drop the panel , run modetest to check the connector status , result connected.
>> Is there any way to detect the connection in this case?
>> Thanks
>
> I am not aware of any DSI drivers that support hotplugging, therefore
> the connector state will always be connected if the device probes.
>
> On vc4 the relevant DSI host controller has to have been enabled in
> device tree and will be a required component for binding. The DSI host
> controller will be waiting on the DSI peripheral driver to call
> mipi_dsi_attach, which then calls component_add. If the panel or panel
> regulator isn't present, then that never happens if the panel isn't
> present, so vc4 won't bind.
> It is a little ugly in that you lose the whole DRM card, but that is
> how I understand DRM is generally set up to work for DSI or similar
> display interfaces.
>
>> -------------------------------------
>>
>> Where did the new timings come from?
>>
>> -------------------------------------
>> My platform dphy tx hardware has certain limitations.
>> Only supports integer multiples of 10M bitrate:
>> such as 160M ,170M, 180M,190M,...1G(max)
>>
>> as common dphy bitrate = pixclock*bpp/lanes.
>> This value cannot match successfully in most cases.
>>
>> so in order to match bitrate , I choose a bitrate value around pixclock*bpp/lanes,
>> Prevent overflow and underflow by fine-tuning the timing parameters:-(
>> that will make the new timming value.
>
> That isn't really a function of the panel then.
>
> All DRM bridges have the option to define a mode_fixup in
> drm_bridge_funcs, and that gives you the option to adjust the timings
> as required.
>
> vc4 has a similar constraint in that the PHY only has an integer
> divider from a 2 or 3GHz PLL. It implements mode_fixup to compute the
> next highest pixel clock, and then adjusts the horizontal front porch
> to keep the same line timing. See
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/vc4/vc4_dsi.c#L825
> I see
I never thought that mode fixup is used to do this.
This would be a correct way to modify its timming parameters

thanks you Dave
> Dave
>
>> >
>> >> Btw there are already DT overlays in mainline which seems to use the
>> >> Raspberry Pi 7inch panel (without touch function yet) [2].
>> >>
>> >> [1] - https://github.com/lategoodbye/rpi-zero/commits/v6.7-7inch-ts
>> >> [2] -
>> >> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x-rpidsi.dtso?h=v6.6.2&id=6b4da1354fd81adace0cda448c77d8f2a47d8474
>> >>
>> >> >
>> >> > Shengyang Chen (2):
>> >> > dt-bindings: display: panel: raspberrypi: Add compatible property for
>> >> > waveshare 7inch touchscreen panel
>> >> > gpu: drm: panel: raspberrypi: add new display mode and new probing
>> >> > process
>> >> >
>> >> > .../panel/raspberrypi,7inch-touchscreen.yaml | 4 +-
>> >> > .../drm/panel/panel-raspberrypi-touchscreen.c | 99 ++++++++++++++++---
>> >> > 2 files changed, 91 insertions(+), 12 deletions(-)
>> >> >
>> >>

2023-12-07 03:52:43

by Shengyang Chen

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] dt-bindings: display: panel: raspberrypi: Add compatible property for waveshare 7inch touchscreen panel

Hi, Conor

thanks for comment

On 2023/12/6 23:40, Conor Dooley wrote:
> On Wed, Dec 06, 2023 at 05:43:48PM +0800, Shengyang Chen wrote:
>> Hi, Conor
>>
>> On 2023/11/24 20:31, Conor Dooley wrote:
>> > On Fri, Nov 24, 2023 at 06:44:50PM +0800, Shengyang Chen wrote:
>> >> The waveshare 7inch touchscreen panel is a kind of raspberrypi pi
>> >> panel
>> >
>> > Can you be more specific about what "is a kind of rpi panel" means?
>> > Are they using identical chips as controllers or something like that?
>> >
>>
>> Wareshare panel has same i2c slave address and registers address with
>> the original raspberry pi panel. They both use Atmel firmware and they
>> got same reg id. It can be operated by using the driver of raspberry pi driver
>> after some change of the code. So I suppose it may be a kind of raspberry pi panel
>> and discribe it in this way. It's my own judgement. Sorry about that.
>> Maybe just like Dave said, It cloned the behaviour of the raspberri pi panel.
>> I will change the discribtion in next version to not make other confused.
>>
>> By the way, we will try Stefan's method before next version.
>> The method we used in this patch may be abandoned if Stefan's method is verified in our platform.
>> At that time yaml may also be changed to fit new method.
>
> I don't know what Stefan's approach is, but I do not think that a
> bindings patch should be dropped. The waveshare might be a clone, but it
> is a distinct device. If the same driver can control both, then the
> compatible setups that should be permitted are:
> compatible = "raspberrypi,7inch-touchscreen-panel";
> and
> compatible = "waveshare,7inch-touchscreen-panel", "raspberrypi,7inch-touchscreen-panel";
>
> Cheers,
> Conor.
>

Here is our consideration of this submit:

Although Waveshare panel reuse the driver of raspberry pi panel, they are different in probing process
and panel parameters.
we try to use compatible and data to distinguish these two panel

Here are the reference
driver part:
https://elixir.bootlin.com/linux/v6.7-rc3/source/drivers/gpu/drm/panel/panel-simple.c
dt-binding part:
https://elixir.bootlin.com/linux/v6.7-rc3/source/Documentation/devicetree/bindings/display/panel/panel-simple.yaml


For example:

in driver part:

in drivers/gpu/drm/panel/panel-simple.c:#in line 4189
----------------------------------------------------------------------------------
we can got different compatible with its own data.
================================================
static const struct of_device_id platform_of_match[] = { //the of_match array list
{
.compatible = "ampire,am-1280800n3tzqw-t00h",
.data = &ampire_am_1280800n3tzqw_t00h, //we define our panel parameter or special panel function, which can distinguish different panels
}, {
.compatible = "ampire,am-480272h3tmqw-t01h",
.data = &ampire_am_480272h3tmqw_t01h,
},
...
...
}
===============================================

in drivers/gpu/drm/panel/panel-simple.c:#in line 4611
----------------------------------------------------------------------------------
we can use the generic probing process to probe our driver
after getting its own data.
================================================
static int panel_simple_platform_probe(struct platform_device *pdev)
{
const struct panel_desc *desc;

desc = of_device_get_match_data(&pdev->dev); //we get our panel parameter
if (!desc)
return -ENODEV;

return panel_simple_probe(&pdev->dev, desc); //probe with returned data
}



================================================

in yamel part:

in /Documentation/devicetree/bindings/display/panel/panel-simple.yaml#in line 33
----------------------------------------------------------------------------------
We refer to this approach, adding our compatible to the yaml of raspberry pi panel
================================================

properties:
compatible:
enum:
# compatible must be listed in alphabetical order, ordered by compatible.
# The description in the comment is mandatory for each compatible.

# Ampire AM-1280800N3TZQW-T00H 10.1" WQVGA TFT LCD panel
- ampire,am-1280800n3tzqw-t00h
# Ampire AM-480272H3TMQW-T01H 4.3" WQVGA TFT LCD panel
- ampire,am-480272h3tmqw-t01h
================================================



If we use Stenfan's method, we can reuse the code of panel-simple.c
we may submit our patch to
/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
/drivers/gpu/drm/panel/panel-simple.c
as a new panel porting. That may less confuse.


here is Stenfan's method:
[1] - https://github.com/lategoodbye/rpi-zero/commits/v6.7-7inch-ts
[2] -
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x-rpidsi.dtso?h=v6.6.2&id=6b4da1354fd81adace0cda448c77d8f2a47d8474


Thanks

Best Regards,
Shengyang


>> >> and it can be drived by panel-raspberrypi-touchscreen.c.
>> >> Add compatible property for it.
>> >>
>> >> Signed-off-by: Keith Zhao <[email protected]>
>> >> Signed-off-by: Shengyang Chen <[email protected]>
>> >> ---
>> >> .../bindings/display/panel/raspberrypi,7inch-touchscreen.yaml | 4 +++-
>> >> 1 file changed, 3 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
>> >> index 22a083f7bc8e..e4e6cb4d4e5b 100644
>> >> --- a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
>> >> +++ b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
>> >> @@ -22,7 +22,9 @@ description: |+
>> >>
>> >> properties:
>> >> compatible:
>> >> - const: raspberrypi,7inch-touchscreen-panel
>> >> + enum:
>> >> + - raspberrypi,7inch-touchscreen-panel
>> >> + - waveshare,7inch-touchscreen-panel
>> >>
>> >> reg:
>> >> const: 0x45
>> >> --
>> >> 2.17.1
>> >>
>>
>>
>> thanks.
>>
>> Best Regards,
>> Shengyang
>>

2023-12-07 09:36:30

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] dt-bindings: display: panel: raspberrypi: Add compatible property for waveshare 7inch touchscreen panel

On Thu, Dec 07, 2023 at 11:48:56AM +0800, Shengyang Chen wrote:
> Hi, Conor
>
> thanks for comment
>
> On 2023/12/6 23:40, Conor Dooley wrote:
> > On Wed, Dec 06, 2023 at 05:43:48PM +0800, Shengyang Chen wrote:
> >> Hi, Conor
> >>
> >> On 2023/11/24 20:31, Conor Dooley wrote:
> >> > On Fri, Nov 24, 2023 at 06:44:50PM +0800, Shengyang Chen wrote:
> >> >> The waveshare 7inch touchscreen panel is a kind of raspberrypi pi
> >> >> panel
> >> >
> >> > Can you be more specific about what "is a kind of rpi panel" means?
> >> > Are they using identical chips as controllers or something like that?
> >> >
> >>
> >> Wareshare panel has same i2c slave address and registers address with
> >> the original raspberry pi panel. They both use Atmel firmware and they
> >> got same reg id. It can be operated by using the driver of raspberry pi driver
> >> after some change of the code. So I suppose it may be a kind of raspberry pi panel
> >> and discribe it in this way. It's my own judgement. Sorry about that.
> >> Maybe just like Dave said, It cloned the behaviour of the raspberri pi panel.
> >> I will change the discribtion in next version to not make other confused.
> >>
> >> By the way, we will try Stefan's method before next version.
> >> The method we used in this patch may be abandoned if Stefan's method is verified in our platform.
> >> At that time yaml may also be changed to fit new method.
> >
> > I don't know what Stefan's approach is, but I do not think that a
> > bindings patch should be dropped. The waveshare might be a clone, but it
> > is a distinct device. If the same driver can control both, then the
> > compatible setups that should be permitted are:
> > compatible = "raspberrypi,7inch-touchscreen-panel";
> > and
> > compatible = "waveshare,7inch-touchscreen-panel", "raspberrypi,7inch-touchscreen-panel";

> If we use Stenfan's method, we can reuse the code of panel-simple.c
> we may submit our patch to
> /Documentation/devicetree/bindings/display/panel/panel-simple.yaml
> /drivers/gpu/drm/panel/panel-simple.c
> as a new panel porting. That may less confuse.

As long as you provide a specific compatible, and not re-use the rpi
one, that's fine. It just sounded like you were intending to reuse that
here, but from this message it seems like I misunderstood.

Thanks,
Conor.


Attachments:
(No filename) (2.32 kB)
signature.asc (235.00 B)
Download all attachments