| Message ID | 20231124104451.44271-1-shengyang.chen@starfivetech.com |
|---|---|
| Headers | show |
| Series | Add waveshare 7inch touchscreen panel support | expand |
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 <keith.zhao@starfivetech.com> > Signed-off-by: Shengyang Chen <shengyang.chen@starfivetech.com> > --- > .../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 >
On Fri, 24 Nov 2023 at 15:00, Stefan Wahren <wahrenst@gmx.net> 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(-) > > >
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
Hi Keith On Wed, 6 Dec 2023 at 08:55, Keith Zhao <keith.zhao@starfivetech.com> wrote: > > > > On 2023/11/25 0:04, Dave Stevenson wrote: > > On Fri, 24 Nov 2023 at 15:00, Stefan Wahren <wahrenst@gmx.net> 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(-) > >> > > >>
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 <keith.zhao@starfivetech.com> > >> Signed-off-by: Shengyang Chen <shengyang.chen@starfivetech.com> > >> --- > >> .../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 >
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(-)