Re: [PATCH v4 7/7] drm/tegra: Add support for 8-bit CPU interface

[Svyatoslav Ryhel]

ср, 28 жовт. 2026 р. о 03:35 Mikko Perttunen <mperttunen@xxxxxxxxxx> пише:
>
> On Sunday, October 25, 2026 5:51 PM Svyatoslav Ryhel wrote:
> > нд, 25 жовт. 2026 р. о 03:46 Mikko Perttunen <mperttunen@xxxxxxxxxx> пише:
> > >
> > > On Friday, October 23, 2026 11:14 PM Svyatoslav Ryhel wrote:
> > > > пт, 23 жовт. 2026 р. о 11:26 Svyatoslav Ryhel <clamor95@xxxxxxxxx> пише:
> > > > >
> > > > > пт, 23 жовт. 2026 р. о 10:53 Mikko Perttunen <mperttunen@xxxxxxxxxx> пише:
> > > > > >
> > > > ...
> > > > > > > +MODULE_ALIAS("platform:tegra-dbi");
> > > > > > > +MODULE_AUTHOR("Svyatoslav Ryhel <clamor95@xxxxxxxxx>");
> > > > > > > +MODULE_DESCRIPTION("Nvidia Tegra20/30 MIPI-DBI Type B driver");
> > > > > > > +MODULE_LICENSE("GPL");
> > > > > >
> > > > > > No separate platform driver or device. If a separate struct device is
> > > > > > needed for some technical purpose, we need to create a minimal separate
> > > > > > virtual device, but unless there is a strong reason to have that we
> > > > > > should be fine with just the dc device.
> > > > > >
> > > > > > Thank you
> > > > > > Mikko
> > > > > >
> > > > >
> > > > > ATM, I use bridge framework to describe interface and I am attaching
> > > > > it to RGB since it basically uses RGB to transfer video data. It works
> > > > > pretty nice so far.
> > > > >
> > > > > You propose to replicate how RGB works just for DBI?
> > > > >
> > > >
> > > > One of the reasons I used separate pdev was to be able get mipi_dbi
> > > > from it using dev_get_drvdata() from patent (dbi bridge in this case).
> > > > DBI framework seems to not have a way to pass it in any other way,
> > > > unless you know smth I don't.
> > >
> > > Right, I think using a bridge is fine; passing the mipi_dbi interface is
> > > properly done in other ways. The direct parent devdata link is a bit of
> > > a "poor man's bus", and sometimes it breaks down, such as here.
> > >
> > > MIPI DBI is comparable to MIPI DSI -- both have a hardware channel that
> > > sends both pixel data and commands. I think we should follow DSI's
> > > example and have a DBI bus and devices while keeping it as simple as
> > > possible.
> > >
> > > The DC driver would register a DBI host and the panel would bind as a
> > > DBI device. The DC driver can then supply the mipi_dbi object in a field
> > > of the mipi_dbi_device.
> > >
> > > This is a bit more complicated than a parent devdata pointer but much
> > > cleaner, and follows the existing DSI pattern.
> > >
> >
> > I understand your intentions, but ATM kernel lacks any DBI type B
> > framework. No hosts, ho devices, noting granular. Implementing
> > everything just not worth my time to support 2 panels of device based
> > on Tegra 20. And I am talking not about time to actually implement
> > this in the code, but to upstream. DSI framework is huge, copying even
> > part of it to wrap legacy DBI interface and ustreaming it does not
> > worth hassle. And yes, I am a poor man, I use a parent devdata bus.
>
> I sketched out the bus implementation and I really don't think it's that
> complicated -- about 200 lines in total. See below. If there turn out to
> be issues we can put an equivalent into drm/tegra/.
>
> -- drm_mipi_dbi_bus.c
>
> /* MIPI DBI devices with command transport through parallel interface */
>
> #include <linux/err.h>
> #include <linux/init.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/slab.h>
>
> #include <drm/drm_mipi_dbi.h>
> #include <drm/drm_mipi_dbi_bus.h>
>
> static int mipi_dbi_match(struct device *dev, const struct device_driver *driver)
> {
>         return of_driver_match_device(dev, driver);
> }
>
> static int mipi_dbi_uevent(const struct device *dev, struct kobj_uevent_env *env)
> {
>         return of_device_uevent_modalias(dev, env);
> }
>
> static const struct bus_type mipi_dbi_bus_type = {
>         .name = "mipi-dbi",
>         .match = mipi_dbi_match,
>         .uevent = mipi_dbi_uevent,
> };
>
> static void mipi_dbi_device_release(struct device *dev)
> {
>         of_node_put(dev->of_node);
>         kfree(to_mipi_dbi_device(dev));
> }
>
> static void mipi_dbi_device_unregister(void *data)
> {
>         struct device *dev = data;
>
>         of_node_clear_flag(dev->of_node, OF_POPULATED);
>         device_unregister(dev);
> }
>
> /* called by dbi host driver to create panel device */
> struct mipi_dbi_device *
> devm_mipi_dbi_device_register(struct mipi_dbi_host *host, struct device_node *np)
> {
>         struct mipi_dbi_device *dbidev;
>         int ret;
>
>         if (!host || !host->dev || !host->dbi || !host->dbi->command ||
>             !host->ops || !host->ops->attach || !host->ops->detach || !np)
>                 return ERR_PTR(-EINVAL);
>
>         if (!of_device_is_available(np))
>                 return ERR_PTR(-ENODEV);
>
>         dbidev = kzalloc_obj(*dbidev);
>         if (!dbidev)
>                 return ERR_PTR(-ENOMEM);
>
>         if (of_node_test_and_set_flag(np, OF_POPULATED)) {
>                 kfree(dbidev);
>                 return ERR_PTR(-EBUSY);
>         }
>
>         dbidev->host = host;
>         dbidev->dbi = host->dbi;
>         device_initialize(&dbidev->dev);
>         dbidev->dev.parent = host->dev;
>         dbidev->dev.bus = &mipi_dbi_bus_type;
>         dbidev->dev.release = mipi_dbi_device_release;
>         device_set_node(&dbidev->dev, of_fwnode_handle(of_node_get(np)));
>
>         ret = dev_set_name(&dbidev->dev, "%s:%s", dev_name(host->dev), np->full_name);
>         if (ret)
>                 goto put_device;
>
>         ret = device_add(&dbidev->dev);
>         if (ret)
>                 goto put_device;
>
>         ret = devm_add_action_or_reset(host->dev, mipi_dbi_device_unregister,
>                                        &dbidev->dev);
>         if (ret)
>                 return ERR_PTR(ret);
>
>         return dbidev;
>
> put_device:
>         of_node_clear_flag(np, OF_POPULATED);
>         put_device(&dbidev->dev);
>         return ERR_PTR(ret);
> }
> EXPORT_SYMBOL_GPL(devm_mipi_dbi_device_register);
>
> static void mipi_dbi_detach(void *data)
> {
>         struct mipi_dbi_device *dev = data;
>
>         dev->host->ops->detach(dev->host, dev);
>         dev->attached = false;
> }
>
> /* called by panel on probe completion to attach to dbi host */
> int devm_mipi_dbi_attach(struct mipi_dbi_device *dev)
> {
>         int ret;
>
>         if (dev->attached)
>                 return -EBUSY;
>
>         ret = dev->host->ops->attach(dev->host, dev);
>         if (ret)
>                 return ret;
>
>         dev->attached = true;
>         return devm_add_action_or_reset(&dev->dev, mipi_dbi_detach, dev);
> }
> EXPORT_SYMBOL_GPL(devm_mipi_dbi_attach);
>
> int __mipi_dbi_driver_register(struct device_driver *driver, struct module *owner)
> {
>         driver->owner = owner;
>         driver->bus = &mipi_dbi_bus_type;
>
>         return driver_register(driver);
> }
> EXPORT_SYMBOL_GPL(__mipi_dbi_driver_register);
>
> void mipi_dbi_driver_unregister(struct device_driver *driver)
> {
>         driver_unregister(driver);
> }
> EXPORT_SYMBOL_GPL(mipi_dbi_driver_unregister);
>
> static int __init mipi_dbi_bus_init(void)
> {
>         return bus_register(&mipi_dbi_bus_type);
> }
> subsys_initcall(mipi_dbi_bus_init);
>
> static void __exit mipi_dbi_bus_exit(void)
> {
>         bus_unregister(&mipi_dbi_bus_type);
> }
> module_exit(mipi_dbi_bus_exit);
>
> -- drm_mipi_dbi_bus.h
>
> #ifndef __DRM_MIPI_DBI_BUS_H__
> #define __DRM_MIPI_DBI_BUS_H__
>
> #include <linux/device.h>
> #include <linux/module.h>
>
> struct device_node;
> struct mipi_dbi;
> struct mipi_dbi_device;
> struct mipi_dbi_host;
>
> struct mipi_dbi_host_ops {
>         int (*attach)(struct mipi_dbi_host *host, struct mipi_dbi_device *dev);
>         void (*detach)(struct mipi_dbi_host *host, struct mipi_dbi_device *dev);
> };
>
> struct mipi_dbi_host {
>         struct device *dev;
>         struct mipi_dbi *dbi;
>         const struct mipi_dbi_host_ops *ops;
> };
>
> struct mipi_dbi_device {
>         struct device dev;
>         struct mipi_dbi *dbi;
>         struct mipi_dbi_host *host;
>         bool attached;
> };
>
> static inline struct mipi_dbi_device *to_mipi_dbi_device(struct device *dev)
> {
>         return container_of(dev, struct mipi_dbi_device, dev);
> }
>
> struct mipi_dbi_device *
> devm_mipi_dbi_device_register(struct mipi_dbi_host *host, struct device_node *np);
> int devm_mipi_dbi_attach(struct mipi_dbi_device *dev);
>
> int __mipi_dbi_driver_register(struct device_driver *driver, struct module *owner);
> void mipi_dbi_driver_unregister(struct device_driver *driver);
>
> #define mipi_dbi_driver_register(driver) \
>         __mipi_dbi_driver_register(driver, THIS_MODULE)
>
> #define module_mipi_dbi_driver(driver) \
>         module_driver(driver, mipi_dbi_driver_register, mipi_dbi_driver_unregister)
>
> #endif
>

Looks promising. I will give it a try.