| Message ID | 1422721984-27782-4-git-send-email-heiko@sntech.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Heiko, Thank you for the patch. On Saturday 31 January 2015 17:32:56 Heiko Stuebner wrote: > There exist simple vga encoders without any type of management interface > and just maybe a simple gpio for turning it on or off. Examples for these > are the Analog Devices ADV7123, Chipsea CS7123 or Micronas SDA7123. > > Add a generic encoder driver for those that can be used by drm drivers > using the component framework. The rcar-du driver already handles the adi,adv7123 compatible string used by this driver. A generic driver is in my opinion a very good idea, but we can't break the existing support. Porting the rcar-du driver to the component model is needed. > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > --- > drivers/gpu/drm/i2c/Kconfig | 6 + > drivers/gpu/drm/i2c/Makefile | 2 + > drivers/gpu/drm/i2c/vga-simple.c | 325 ++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i2c/ feels wrong for a platform device. > 3 files changed, 333 insertions(+) > create mode 100644 drivers/gpu/drm/i2c/vga-simple.c > > diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig > index 22c7ed6..319f2cb 100644 > --- a/drivers/gpu/drm/i2c/Kconfig > +++ b/drivers/gpu/drm/i2c/Kconfig > @@ -31,4 +31,10 @@ config DRM_I2C_NXP_TDA998X > help > Support for NXP Semiconductors TDA998X HDMI encoders. > > +config DRM_VGA_SIMPLE > + tristate "Generic simple vga encoder" > + help > + Support for vga encoder chips without any special settings > + and at most a power-control gpio. > + > endmenu > diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile > index 2c72eb5..858961f 100644 > --- a/drivers/gpu/drm/i2c/Makefile > +++ b/drivers/gpu/drm/i2c/Makefile > @@ -10,3 +10,5 @@ obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o > > tda998x-y := tda998x_drv.o > obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o > + > +obj-$(CONFIG_DRM_VGA_SIMPLE) += vga-simple.o > diff --git a/drivers/gpu/drm/i2c/vga-simple.c > b/drivers/gpu/drm/i2c/vga-simple.c new file mode 100644 > index 0000000..bb9d19c > --- /dev/null > +++ b/drivers/gpu/drm/i2c/vga-simple.c > @@ -0,0 +1,325 @@ > +/* > + * Simple vga encoder driver > + * > + * Copyright (C) 2014 Heiko Stuebner <heiko@sntech.de> > + * > + * This software is licensed under the terms of the GNU General Public > + * License version 2, as published by the Free Software Foundation, and > + * may be copied, distributed, and modified under those terms. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <linux/module.h> > +#include <linux/component.h> > +#include <linux/gpio.h> > +#include <linux/gpio/consumer.h> > +#include <linux/i2c.h> > +#include <linux/regulator/consumer.h> > +#include <drm/drm_of.h> > +#include <drm/drmP.h> > +#include <drm/drm_crtc.h> > +#include <drm/drm_crtc_helper.h> > +#include <drm/drm_edid.h> > + > +#define connector_to_vga_simple(x) container_of(x, struct vga_simple, > connector) +#define encoder_to_vga_simple(x) container_of(x, struct > vga_simple, encoder) + > +struct vga_simple { > + struct drm_connector connector; > + struct drm_encoder encoder; > + > + struct device *dev; > + struct i2c_adapter *ddc; > + > + struct regulator *vaa_reg; > + struct gpio_desc *enable_gpio; > + > + struct mutex enable_lock; > + bool enabled; > +}; > + > +/* > + * Connector functions > + */ > + > +enum drm_connector_status vga_simple_detect(struct drm_connector > *connector, + bool force) > +{ > + struct vga_simple *vga = connector_to_vga_simple(connector); > + > + if (!vga->ddc) > + return connector_status_unknown; > + > + if (drm_probe_ddc(vga->ddc)) > + return connector_status_connected; > + > + return connector_status_disconnected; > +} > + > +void vga_simple_connector_destroy(struct drm_connector *connector) > +{ > + drm_connector_unregister(connector); > + drm_connector_cleanup(connector); > +} > + > +struct drm_connector_funcs vga_simple_connector_funcs = { > + .dpms = drm_helper_connector_dpms, > + .fill_modes = drm_helper_probe_single_connector_modes, > + .detect = vga_simple_detect, > + .destroy = vga_simple_connector_destroy, > +}; > + > +/* > + * Connector helper functions > + */ > + > +static int vga_simple_connector_get_modes(struct drm_connector *connector) > +{ > + struct vga_simple *vga = connector_to_vga_simple(connector); > + struct edid *edid; > + int ret = 0; > + > + if (!vga->ddc) > + return 0; > + > + edid = drm_get_edid(connector, vga->ddc); > + if (edid) { > + drm_mode_connector_update_edid_property(connector, edid); > + ret = drm_add_edid_modes(connector, edid); > + kfree(edid); > + } > + > + return ret; > +} > + > +static int vga_simple_connector_mode_valid(struct drm_connector *connector, > + struct drm_display_mode *mode) > +{ > + return MODE_OK; > +} > + > +static struct drm_encoder > +*vga_simple_connector_best_encoder(struct drm_connector *connector) > +{ > + struct vga_simple *vga = connector_to_vga_simple(connector); > + > + return &vga->encoder; > +} > + > +static struct drm_connector_helper_funcs vga_simple_connector_helper_funcs > = { + .get_modes = vga_simple_connector_get_modes, > + .best_encoder = vga_simple_connector_best_encoder, > + .mode_valid = vga_simple_connector_mode_valid, > +}; > + > +/* > + * Encoder functions > + */ > + > +static void vga_simple_encoder_destroy(struct drm_encoder *encoder) > +{ > + drm_encoder_cleanup(encoder); > +} > + > +static const struct drm_encoder_funcs vga_simple_encoder_funcs = { > + .destroy = vga_simple_encoder_destroy, > +}; > + > +/* > + * Encoder helper functions > + */ > + > +static void vga_simple_encoder_dpms(struct drm_encoder *encoder, int mode) > +{ > + struct vga_simple *vga = encoder_to_vga_simple(encoder); > + > + mutex_lock(&vga->enable_lock); > + > + switch (mode) { > + case DRM_MODE_DPMS_ON: > + if (vga->enabled) > + goto out; > + > + if (!IS_ERR(vga->vaa_reg)) > + regulator_enable(vga->vaa_reg); > + > + if (vga->enable_gpio) > + gpiod_set_value(vga->enable_gpio, 1); > + > + vga->enabled = true; > + break; > + case DRM_MODE_DPMS_STANDBY: > + case DRM_MODE_DPMS_SUSPEND: > + case DRM_MODE_DPMS_OFF: > + if (!vga->enabled) > + goto out; > + > + if (vga->enable_gpio) > + gpiod_set_value(vga->enable_gpio, 0); > + > + if (!IS_ERR(vga->vaa_reg)) > + regulator_enable(vga->vaa_reg); > + > + vga->enabled = false; > + break; > + default: > + break; > + } > + > +out: > + mutex_unlock(&vga->enable_lock); > +} > + > +static bool vga_simple_mode_fixup(struct drm_encoder *encoder, > + const struct drm_display_mode *mode, > + struct drm_display_mode *adjusted_mode) > +{ > + return true; > +} > + > +static void vga_simple_encoder_prepare(struct drm_encoder *encoder) > +{ > +} > + > +static void vga_simple_encoder_mode_set(struct drm_encoder *encoder, > + struct drm_display_mode *mode, > + struct drm_display_mode *adjusted_mode) > +{ > +} > + > +static void vga_simple_encoder_commit(struct drm_encoder *encoder) > +{ > + vga_simple_encoder_dpms(encoder, DRM_MODE_DPMS_ON); > +} > + > +static void vga_simple_encoder_disable(struct drm_encoder *encoder) > +{ > + vga_simple_encoder_dpms(encoder, DRM_MODE_DPMS_OFF); > +} > + > +static const struct drm_encoder_helper_funcs > vga_simple_encoder_helper_funcs = { > + .dpms = vga_simple_encoder_dpms, > + .mode_fixup = vga_simple_mode_fixup, > + .prepare = vga_simple_encoder_prepare, > + .mode_set = vga_simple_encoder_mode_set, > + .commit = vga_simple_encoder_commit, > + .disable = vga_simple_encoder_disable, this is interesting. Some users of this encoder (I'm thinking about rcar-du, for which I've just posted the patches) are already ported to atomic updates, while others are not. How can we support both ? > +}; > + > +/* > + * Component helper functions > + */ > + > +static int vga_simple_bind(struct device *dev, struct device *master, > + void *data) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + struct drm_device *drm = data; > + > + struct device_node *ddc_node, *np = pdev->dev.of_node; > + struct vga_simple *vga; > + int ret; > + > + if (!np) > + return -ENODEV; > + > + vga = devm_kzalloc(dev, sizeof(*vga), GFP_KERNEL); > + if (!vga) > + return -ENOMEM; > + > + vga->dev = dev; > + dev_set_drvdata(dev, vga); > + mutex_init(&vga->enable_lock); > + > + vga->enable_gpio = devm_gpiod_get_optional(dev, "enable", > + GPIOD_OUT_LOW); > + if (IS_ERR(vga->enable_gpio)) { > + ret = PTR_ERR(vga->enable_gpio); > + dev_err(dev, "failed to request GPIO: %d\n", ret); > + return ret; > + } > + > + vga->enabled = false; > + > + ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); > + if (ddc_node) { > + vga->ddc = of_find_i2c_adapter_by_node(ddc_node); > + of_node_put(ddc_node); > + if (!vga->ddc) { > + dev_dbg(vga->dev, "failed to read ddc node\n"); > + return -EPROBE_DEFER; > + } > + } else { > + dev_dbg(vga->dev, "no ddc property found\n"); > + } > + > + vga->vaa_reg = devm_regulator_get_optional(dev, "vaa"); > + > + vga->encoder.possible_crtcs = drm_of_find_possible_crtcs(drm, np); > + vga->encoder.of_node = np; > + vga->connector.polled = DRM_CONNECTOR_POLL_CONNECT | > + DRM_CONNECTOR_POLL_DISCONNECT; > + > + drm_encoder_helper_add(&vga->encoder, &vga_simple_encoder_helper_funcs); > + drm_encoder_init(drm, &vga->encoder, &vga_simple_encoder_funcs, > + DRM_MODE_ENCODER_DAC); > + > + drm_connector_helper_add(&vga->connector, > + &vga_simple_connector_helper_funcs); I really dislike this, this is an encoder driver, not a connector driver. It shouldn't be responsible for creating the connector. For all it knows, there might not even be a VGA connector connected to the encoder, VGA signals could be sent to a chained encoder. That might be a bit far-fetched in the VGA case, but in the generic case encoder drivers shouldn't create connectors. > + drm_connector_init(drm, &vga->connector, &vga_simple_connector_funcs, > + DRM_MODE_CONNECTOR_VGA); > + > + drm_mode_connector_attach_encoder(&vga->connector, &vga->encoder); > + > + return 0; > +} > + > +static void vga_simple_unbind(struct device *dev, struct device *master, > + void *data) > +{ > + struct vga_simple *vga = dev_get_drvdata(dev); > + > + vga->connector.funcs->destroy(&vga->connector); > + vga->encoder.funcs->destroy(&vga->encoder); > +} > + > +static const struct component_ops vga_simple_ops = { > + .bind = vga_simple_bind, > + .unbind = vga_simple_unbind, > +}; > + > +static int vga_simple_probe(struct platform_device *pdev) > +{ > + return component_add(&pdev->dev, &vga_simple_ops); > +} > + > +static int vga_simple_remove(struct platform_device *pdev) > +{ > + component_del(&pdev->dev, &vga_simple_ops); > + > + return 0; > +} > + > +static const struct of_device_id vga_simple_ids[] = { > + { .compatible = "adi,adv7123", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, vga_simple_ids); > + > +static struct platform_driver vga_simple_driver = { > + .probe = vga_simple_probe, > + .remove = vga_simple_remove, > + .driver = { > + .name = "vga-simple", > + .of_match_table = vga_simple_ids, > + }, > +}; > +module_platform_driver(vga_simple_driver); > + > +MODULE_AUTHOR("Heiko Stuebner <heiko@sntech.de>"); > +MODULE_DESCRIPTION("Simple vga converter"); > +MODULE_LICENSE("GPL");
On Sat, Jan 31, 2015 at 10:32 AM, Heiko Stuebner <heiko@sntech.de> wrote: > There exist simple vga encoders without any type of management interface > and just maybe a simple gpio for turning it on or off. Examples for these > are the Analog Devices ADV7123, Chipsea CS7123 or Micronas SDA7123. > > Add a generic encoder driver for those that can be used by drm drivers > using the component framework. What makes this specific to VGA other than DRM_MODE_CONNECTOR_VGA? Seems like with match data you could generalize this to any simple encoder chip. Rob > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > --- > drivers/gpu/drm/i2c/Kconfig | 6 + > drivers/gpu/drm/i2c/Makefile | 2 + > drivers/gpu/drm/i2c/vga-simple.c | 325 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 333 insertions(+) > create mode 100644 drivers/gpu/drm/i2c/vga-simple.c > > diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig > index 22c7ed6..319f2cb 100644 > --- a/drivers/gpu/drm/i2c/Kconfig > +++ b/drivers/gpu/drm/i2c/Kconfig > @@ -31,4 +31,10 @@ config DRM_I2C_NXP_TDA998X > help > Support for NXP Semiconductors TDA998X HDMI encoders. > > +config DRM_VGA_SIMPLE > + tristate "Generic simple vga encoder" > + help > + Support for vga encoder chips without any special settings > + and at most a power-control gpio. > + > endmenu > diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile > index 2c72eb5..858961f 100644 > --- a/drivers/gpu/drm/i2c/Makefile > +++ b/drivers/gpu/drm/i2c/Makefile > @@ -10,3 +10,5 @@ obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o > > tda998x-y := tda998x_drv.o > obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o > + > +obj-$(CONFIG_DRM_VGA_SIMPLE) += vga-simple.o > diff --git a/drivers/gpu/drm/i2c/vga-simple.c b/drivers/gpu/drm/i2c/vga-simple.c > new file mode 100644 > index 0000000..bb9d19c > --- /dev/null > +++ b/drivers/gpu/drm/i2c/vga-simple.c > @@ -0,0 +1,325 @@ > +/* > + * Simple vga encoder driver > + * > + * Copyright (C) 2014 Heiko Stuebner <heiko@sntech.de> > + * > + * This software is licensed under the terms of the GNU General Public > + * License version 2, as published by the Free Software Foundation, and > + * may be copied, distributed, and modified under those terms. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <linux/module.h> > +#include <linux/component.h> > +#include <linux/gpio.h> > +#include <linux/gpio/consumer.h> > +#include <linux/i2c.h> > +#include <linux/regulator/consumer.h> > +#include <drm/drm_of.h> > +#include <drm/drmP.h> > +#include <drm/drm_crtc.h> > +#include <drm/drm_crtc_helper.h> > +#include <drm/drm_edid.h> > + > +#define connector_to_vga_simple(x) container_of(x, struct vga_simple, connector) > +#define encoder_to_vga_simple(x) container_of(x, struct vga_simple, encoder) > + > +struct vga_simple { > + struct drm_connector connector; > + struct drm_encoder encoder; > + > + struct device *dev; > + struct i2c_adapter *ddc; > + > + struct regulator *vaa_reg; > + struct gpio_desc *enable_gpio; > + > + struct mutex enable_lock; > + bool enabled; > +}; > + > +/* > + * Connector functions > + */ > + > +enum drm_connector_status vga_simple_detect(struct drm_connector *connector, > + bool force) > +{ > + struct vga_simple *vga = connector_to_vga_simple(connector); > + > + if (!vga->ddc) > + return connector_status_unknown; > + > + if (drm_probe_ddc(vga->ddc)) > + return connector_status_connected; > + > + return connector_status_disconnected; > +} > + > +void vga_simple_connector_destroy(struct drm_connector *connector) > +{ > + drm_connector_unregister(connector); > + drm_connector_cleanup(connector); > +} > + > +struct drm_connector_funcs vga_simple_connector_funcs = { > + .dpms = drm_helper_connector_dpms, > + .fill_modes = drm_helper_probe_single_connector_modes, > + .detect = vga_simple_detect, > + .destroy = vga_simple_connector_destroy, > +}; > + > +/* > + * Connector helper functions > + */ > + > +static int vga_simple_connector_get_modes(struct drm_connector *connector) > +{ > + struct vga_simple *vga = connector_to_vga_simple(connector); > + struct edid *edid; > + int ret = 0; > + > + if (!vga->ddc) > + return 0; > + > + edid = drm_get_edid(connector, vga->ddc); > + if (edid) { > + drm_mode_connector_update_edid_property(connector, edid); > + ret = drm_add_edid_modes(connector, edid); > + kfree(edid); > + } > + > + return ret; > +} > + > +static int vga_simple_connector_mode_valid(struct drm_connector *connector, > + struct drm_display_mode *mode) > +{ > + return MODE_OK; > +} > + > +static struct drm_encoder > +*vga_simple_connector_best_encoder(struct drm_connector *connector) > +{ > + struct vga_simple *vga = connector_to_vga_simple(connector); > + > + return &vga->encoder; > +} > + > +static struct drm_connector_helper_funcs vga_simple_connector_helper_funcs = { > + .get_modes = vga_simple_connector_get_modes, > + .best_encoder = vga_simple_connector_best_encoder, > + .mode_valid = vga_simple_connector_mode_valid, > +}; > + > +/* > + * Encoder functions > + */ > + > +static void vga_simple_encoder_destroy(struct drm_encoder *encoder) > +{ > + drm_encoder_cleanup(encoder); > +} > + > +static const struct drm_encoder_funcs vga_simple_encoder_funcs = { > + .destroy = vga_simple_encoder_destroy, > +}; > + > +/* > + * Encoder helper functions > + */ > + > +static void vga_simple_encoder_dpms(struct drm_encoder *encoder, int mode) > +{ > + struct vga_simple *vga = encoder_to_vga_simple(encoder); > + > + mutex_lock(&vga->enable_lock); > + > + switch (mode) { > + case DRM_MODE_DPMS_ON: > + if (vga->enabled) > + goto out; > + > + if (!IS_ERR(vga->vaa_reg)) > + regulator_enable(vga->vaa_reg); > + > + if (vga->enable_gpio) > + gpiod_set_value(vga->enable_gpio, 1); > + > + vga->enabled = true; > + break; > + case DRM_MODE_DPMS_STANDBY: > + case DRM_MODE_DPMS_SUSPEND: > + case DRM_MODE_DPMS_OFF: > + if (!vga->enabled) > + goto out; > + > + if (vga->enable_gpio) > + gpiod_set_value(vga->enable_gpio, 0); > + > + if (!IS_ERR(vga->vaa_reg)) > + regulator_enable(vga->vaa_reg); > + > + vga->enabled = false; > + break; > + default: > + break; > + } > + > +out: > + mutex_unlock(&vga->enable_lock); > +} > + > +static bool vga_simple_mode_fixup(struct drm_encoder *encoder, > + const struct drm_display_mode *mode, > + struct drm_display_mode *adjusted_mode) > +{ > + return true; > +} > + > +static void vga_simple_encoder_prepare(struct drm_encoder *encoder) > +{ > +} > + > +static void vga_simple_encoder_mode_set(struct drm_encoder *encoder, > + struct drm_display_mode *mode, > + struct drm_display_mode *adjusted_mode) > +{ > +} > + > +static void vga_simple_encoder_commit(struct drm_encoder *encoder) > +{ > + vga_simple_encoder_dpms(encoder, DRM_MODE_DPMS_ON); > +} > + > +static void vga_simple_encoder_disable(struct drm_encoder *encoder) > +{ > + vga_simple_encoder_dpms(encoder, DRM_MODE_DPMS_OFF); > +} > + > +static const struct drm_encoder_helper_funcs vga_simple_encoder_helper_funcs = { > + .dpms = vga_simple_encoder_dpms, > + .mode_fixup = vga_simple_mode_fixup, > + .prepare = vga_simple_encoder_prepare, > + .mode_set = vga_simple_encoder_mode_set, > + .commit = vga_simple_encoder_commit, > + .disable = vga_simple_encoder_disable, > +}; > + > +/* > + * Component helper functions > + */ > + > +static int vga_simple_bind(struct device *dev, struct device *master, > + void *data) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + struct drm_device *drm = data; > + > + struct device_node *ddc_node, *np = pdev->dev.of_node; > + struct vga_simple *vga; > + int ret; > + > + if (!np) > + return -ENODEV; > + > + vga = devm_kzalloc(dev, sizeof(*vga), GFP_KERNEL); > + if (!vga) > + return -ENOMEM; > + > + vga->dev = dev; > + dev_set_drvdata(dev, vga); > + mutex_init(&vga->enable_lock); > + > + vga->enable_gpio = devm_gpiod_get_optional(dev, "enable", > + GPIOD_OUT_LOW); > + if (IS_ERR(vga->enable_gpio)) { > + ret = PTR_ERR(vga->enable_gpio); > + dev_err(dev, "failed to request GPIO: %d\n", ret); > + return ret; > + } > + > + vga->enabled = false; > + > + ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); > + if (ddc_node) { > + vga->ddc = of_find_i2c_adapter_by_node(ddc_node); > + of_node_put(ddc_node); > + if (!vga->ddc) { > + dev_dbg(vga->dev, "failed to read ddc node\n"); > + return -EPROBE_DEFER; > + } > + } else { > + dev_dbg(vga->dev, "no ddc property found\n"); > + } > + > + vga->vaa_reg = devm_regulator_get_optional(dev, "vaa"); > + > + vga->encoder.possible_crtcs = drm_of_find_possible_crtcs(drm, np); > + vga->encoder.of_node = np; > + vga->connector.polled = DRM_CONNECTOR_POLL_CONNECT | > + DRM_CONNECTOR_POLL_DISCONNECT; > + > + drm_encoder_helper_add(&vga->encoder, &vga_simple_encoder_helper_funcs); > + drm_encoder_init(drm, &vga->encoder, &vga_simple_encoder_funcs, > + DRM_MODE_ENCODER_DAC); > + > + drm_connector_helper_add(&vga->connector, > + &vga_simple_connector_helper_funcs); > + drm_connector_init(drm, &vga->connector, &vga_simple_connector_funcs, > + DRM_MODE_CONNECTOR_VGA); > + > + drm_mode_connector_attach_encoder(&vga->connector, &vga->encoder); > + > + return 0; > +} > + > +static void vga_simple_unbind(struct device *dev, struct device *master, > + void *data) > +{ > + struct vga_simple *vga = dev_get_drvdata(dev); > + > + vga->connector.funcs->destroy(&vga->connector); > + vga->encoder.funcs->destroy(&vga->encoder); > +} > + > +static const struct component_ops vga_simple_ops = { > + .bind = vga_simple_bind, > + .unbind = vga_simple_unbind, > +}; > + > +static int vga_simple_probe(struct platform_device *pdev) > +{ > + return component_add(&pdev->dev, &vga_simple_ops); > +} > + > +static int vga_simple_remove(struct platform_device *pdev) > +{ > + component_del(&pdev->dev, &vga_simple_ops); > + > + return 0; > +} > + > +static const struct of_device_id vga_simple_ids[] = { > + { .compatible = "adi,adv7123", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, vga_simple_ids); > + > +static struct platform_driver vga_simple_driver = { > + .probe = vga_simple_probe, > + .remove = vga_simple_remove, > + .driver = { > + .name = "vga-simple", > + .of_match_table = vga_simple_ids, > + }, > +}; > +module_platform_driver(vga_simple_driver); > + > +MODULE_AUTHOR("Heiko Stuebner <heiko@sntech.de>"); > +MODULE_DESCRIPTION("Simple vga converter"); > +MODULE_LICENSE("GPL"); > -- > 2.1.1 >
Hi Laurent, thanks for the comments Am Donnerstag, 26. Februar 2015, 20:33:33 schrieb Laurent Pinchart: > On Saturday 31 January 2015 17:32:56 Heiko Stuebner wrote: > > There exist simple vga encoders without any type of management interface > > and just maybe a simple gpio for turning it on or off. Examples for these > > are the Analog Devices ADV7123, Chipsea CS7123 or Micronas SDA7123. > > > > Add a generic encoder driver for those that can be used by drm drivers > > using the component framework. > > The rcar-du driver already handles the adi,adv7123 compatible string used by > this driver. A generic driver is in my opinion a very good idea, but we > can't break the existing support. Porting the rcar-du driver to the > component model is needed. is this in someones plans already? Because I'm still having trouble understanding parts of the rockchip driver sometimes, so feel in no way qualified to try to get this tackled in some reasonable timeframe :-) > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > > --- > > > > drivers/gpu/drm/i2c/Kconfig | 6 + > > drivers/gpu/drm/i2c/Makefile | 2 + > > drivers/gpu/drm/i2c/vga-simple.c | 325 > > ++++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/i2c/ feels wrong for a platform device. yep, i2c probably being the wrong place was one of the caveats in the cover letter and I was hoping some more seasoned drm people could suggest where this should live after all. As your comments further down suggest that we might introduce some different components, simply congregate them in a "components" subdir or something splitting them more? > > > 3 files changed, 333 insertions(+) > > create mode 100644 drivers/gpu/drm/i2c/vga-simple.c > > > > diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig > > index 22c7ed6..319f2cb 100644 > > --- a/drivers/gpu/drm/i2c/Kconfig > > +++ b/drivers/gpu/drm/i2c/Kconfig > > @@ -31,4 +31,10 @@ config DRM_I2C_NXP_TDA998X > > > > help > > > > Support for NXP Semiconductors TDA998X HDMI encoders. > > > > +config DRM_VGA_SIMPLE > > + tristate "Generic simple vga encoder" > > + help > > + Support for vga encoder chips without any special settings > > + and at most a power-control gpio. > > + > > > > endmenu > > > > diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile > > index 2c72eb5..858961f 100644 > > --- a/drivers/gpu/drm/i2c/Makefile > > +++ b/drivers/gpu/drm/i2c/Makefile > > @@ -10,3 +10,5 @@ obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o > > > > tda998x-y := tda998x_drv.o > > obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o > > > > + > > +obj-$(CONFIG_DRM_VGA_SIMPLE) += vga-simple.o > > diff --git a/drivers/gpu/drm/i2c/vga-simple.c > > b/drivers/gpu/drm/i2c/vga-simple.c new file mode 100644 > > index 0000000..bb9d19c > > --- /dev/null > > +++ b/drivers/gpu/drm/i2c/vga-simple.c > > @@ -0,0 +1,325 @@ > > +/* > > + * Simple vga encoder driver > > + * > > + * Copyright (C) 2014 Heiko Stuebner <heiko@sntech.de> > > + * > > + * This software is licensed under the terms of the GNU General Public > > + * License version 2, as published by the Free Software Foundation, and > > + * may be copied, distributed, and modified under those terms. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + */ > > + > > +#include <linux/module.h> > > +#include <linux/component.h> > > +#include <linux/gpio.h> > > +#include <linux/gpio/consumer.h> > > +#include <linux/i2c.h> > > +#include <linux/regulator/consumer.h> > > +#include <drm/drm_of.h> > > +#include <drm/drmP.h> > > +#include <drm/drm_crtc.h> > > +#include <drm/drm_crtc_helper.h> > > +#include <drm/drm_edid.h> > > + > > +#define connector_to_vga_simple(x) container_of(x, struct vga_simple, > > connector) +#define encoder_to_vga_simple(x) container_of(x, struct > > vga_simple, encoder) + > > +struct vga_simple { > > + struct drm_connector connector; > > + struct drm_encoder encoder; > > + > > + struct device *dev; > > + struct i2c_adapter *ddc; > > + > > + struct regulator *vaa_reg; > > + struct gpio_desc *enable_gpio; > > + > > + struct mutex enable_lock; > > + bool enabled; > > +}; > > + > > +/* > > + * Connector functions > > + */ > > + > > +enum drm_connector_status vga_simple_detect(struct drm_connector > > *connector, + bool force) > > +{ > > + struct vga_simple *vga = connector_to_vga_simple(connector); > > + > > + if (!vga->ddc) > > + return connector_status_unknown; > > + > > + if (drm_probe_ddc(vga->ddc)) > > + return connector_status_connected; > > + > > + return connector_status_disconnected; > > +} > > + > > +void vga_simple_connector_destroy(struct drm_connector *connector) > > +{ > > + drm_connector_unregister(connector); > > + drm_connector_cleanup(connector); > > +} > > + > > +struct drm_connector_funcs vga_simple_connector_funcs = { > > + .dpms = drm_helper_connector_dpms, > > + .fill_modes = drm_helper_probe_single_connector_modes, > > + .detect = vga_simple_detect, > > + .destroy = vga_simple_connector_destroy, > > +}; > > + > > +/* > > + * Connector helper functions > > + */ > > + > > +static int vga_simple_connector_get_modes(struct drm_connector > > *connector) > > +{ > > + struct vga_simple *vga = connector_to_vga_simple(connector); > > + struct edid *edid; > > + int ret = 0; > > + > > + if (!vga->ddc) > > + return 0; > > + > > + edid = drm_get_edid(connector, vga->ddc); > > + if (edid) { > > + drm_mode_connector_update_edid_property(connector, edid); > > + ret = drm_add_edid_modes(connector, edid); > > + kfree(edid); > > + } > > + > > + return ret; > > +} > > + > > +static int vga_simple_connector_mode_valid(struct drm_connector > > *connector, + struct drm_display_mode *mode) > > +{ > > + return MODE_OK; > > +} > > + > > +static struct drm_encoder > > +*vga_simple_connector_best_encoder(struct drm_connector *connector) > > +{ > > + struct vga_simple *vga = connector_to_vga_simple(connector); > > + > > + return &vga->encoder; > > +} > > + > > +static struct drm_connector_helper_funcs > > vga_simple_connector_helper_funcs > > = { + .get_modes = vga_simple_connector_get_modes, > > + .best_encoder = vga_simple_connector_best_encoder, > > + .mode_valid = vga_simple_connector_mode_valid, > > +}; > > + > > +/* > > + * Encoder functions > > + */ > > + > > +static void vga_simple_encoder_destroy(struct drm_encoder *encoder) > > +{ > > + drm_encoder_cleanup(encoder); > > +} > > + > > +static const struct drm_encoder_funcs vga_simple_encoder_funcs = { > > + .destroy = vga_simple_encoder_destroy, > > +}; > > + > > +/* > > + * Encoder helper functions > > + */ > > + > > +static void vga_simple_encoder_dpms(struct drm_encoder *encoder, int > > mode) > > +{ > > + struct vga_simple *vga = encoder_to_vga_simple(encoder); > > + > > + mutex_lock(&vga->enable_lock); > > + > > + switch (mode) { > > + case DRM_MODE_DPMS_ON: > > + if (vga->enabled) > > + goto out; > > + > > + if (!IS_ERR(vga->vaa_reg)) > > + regulator_enable(vga->vaa_reg); > > + > > + if (vga->enable_gpio) > > + gpiod_set_value(vga->enable_gpio, 1); > > + > > + vga->enabled = true; > > + break; > > + case DRM_MODE_DPMS_STANDBY: > > + case DRM_MODE_DPMS_SUSPEND: > > + case DRM_MODE_DPMS_OFF: > > + if (!vga->enabled) > > + goto out; > > + > > + if (vga->enable_gpio) > > + gpiod_set_value(vga->enable_gpio, 0); > > + > > + if (!IS_ERR(vga->vaa_reg)) > > + regulator_enable(vga->vaa_reg); > > + > > + vga->enabled = false; > > + break; > > + default: > > + break; > > + } > > + > > +out: > > + mutex_unlock(&vga->enable_lock); > > +} > > + > > +static bool vga_simple_mode_fixup(struct drm_encoder *encoder, > > + const struct drm_display_mode *mode, > > + struct drm_display_mode *adjusted_mode) > > +{ > > + return true; > > +} > > + > > +static void vga_simple_encoder_prepare(struct drm_encoder *encoder) > > +{ > > +} > > + > > +static void vga_simple_encoder_mode_set(struct drm_encoder *encoder, > > + struct drm_display_mode *mode, > > + struct drm_display_mode *adjusted_mode) > > +{ > > +} > > + > > +static void vga_simple_encoder_commit(struct drm_encoder *encoder) > > +{ > > + vga_simple_encoder_dpms(encoder, DRM_MODE_DPMS_ON); > > +} > > + > > +static void vga_simple_encoder_disable(struct drm_encoder *encoder) > > +{ > > + vga_simple_encoder_dpms(encoder, DRM_MODE_DPMS_OFF); > > +} > > + > > +static const struct drm_encoder_helper_funcs > > vga_simple_encoder_helper_funcs = { > > + .dpms = vga_simple_encoder_dpms, > > + .mode_fixup = vga_simple_mode_fixup, > > + .prepare = vga_simple_encoder_prepare, > > + .mode_set = vga_simple_encoder_mode_set, > > + .commit = vga_simple_encoder_commit, > > + .disable = vga_simple_encoder_disable, > > this is interesting. Some users of this encoder (I'm thinking about rcar-du, > for which I've just posted the patches) are already ported to atomic > updates, while others are not. How can we support both ? Wild guess, support both in such generic components and let bind decide which to use ... is there some sort of drm_is_atomic() against the master device. > > > +}; > > + > > +/* > > + * Component helper functions > > + */ > > + > > +static int vga_simple_bind(struct device *dev, struct device *master, > > + void *data) > > +{ > > + struct platform_device *pdev = to_platform_device(dev); > > + struct drm_device *drm = data; > > + > > + struct device_node *ddc_node, *np = pdev->dev.of_node; > > + struct vga_simple *vga; > > + int ret; > > + > > + if (!np) > > + return -ENODEV; > > + > > + vga = devm_kzalloc(dev, sizeof(*vga), GFP_KERNEL); > > + if (!vga) > > + return -ENOMEM; > > + > > + vga->dev = dev; > > + dev_set_drvdata(dev, vga); > > + mutex_init(&vga->enable_lock); > > + > > + vga->enable_gpio = devm_gpiod_get_optional(dev, "enable", > > + GPIOD_OUT_LOW); > > + if (IS_ERR(vga->enable_gpio)) { > > + ret = PTR_ERR(vga->enable_gpio); > > + dev_err(dev, "failed to request GPIO: %d\n", ret); > > + return ret; > > + } > > + > > + vga->enabled = false; > > + > > + ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); > > + if (ddc_node) { > > + vga->ddc = of_find_i2c_adapter_by_node(ddc_node); > > + of_node_put(ddc_node); > > + if (!vga->ddc) { > > + dev_dbg(vga->dev, "failed to read ddc node\n"); > > + return -EPROBE_DEFER; > > + } > > + } else { > > + dev_dbg(vga->dev, "no ddc property found\n"); > > + } > > + > > + vga->vaa_reg = devm_regulator_get_optional(dev, "vaa"); > > + > > + vga->encoder.possible_crtcs = drm_of_find_possible_crtcs(drm, np); > > + vga->encoder.of_node = np; > > + vga->connector.polled = DRM_CONNECTOR_POLL_CONNECT | > > + DRM_CONNECTOR_POLL_DISCONNECT; > > + > > + drm_encoder_helper_add(&vga->encoder, &vga_simple_encoder_helper_funcs); > > + drm_encoder_init(drm, &vga->encoder, &vga_simple_encoder_funcs, > > + DRM_MODE_ENCODER_DAC); > > + > > + drm_connector_helper_add(&vga->connector, > > + &vga_simple_connector_helper_funcs); > > I really dislike this, this is an encoder driver, not a connector driver. It > shouldn't be responsible for creating the connector. For all it knows, > there might not even be a VGA connector connected to the encoder, VGA > signals could be sent to a chained encoder. That might be a bit far-fetched > in the VGA case, but in the generic case encoder drivers shouldn't create > connectors. ok, so you suggest to have this split. Taking into account Rob's comment probably a "simple-encoder" and a "ddc-connector" for connectors using system i2c busses? And then connect everything via ports. Thanks Heiko
Hi Laurent, Am Samstag, 28. Februar 2015, 01:42:45 schrieb Heiko Stübner: > thanks for the comments > > Am Donnerstag, 26. Februar 2015, 20:33:33 schrieb Laurent Pinchart: > > On Saturday 31 January 2015 17:32:56 Heiko Stuebner wrote: > > > There exist simple vga encoders without any type of management interface > > > and just maybe a simple gpio for turning it on or off. Examples for > > > these > > > are the Analog Devices ADV7123, Chipsea CS7123 or Micronas SDA7123. > > > > > > Add a generic encoder driver for those that can be used by drm drivers > > > using the component framework. > > > > The rcar-du driver already handles the adi,adv7123 compatible string used > > by this driver. A generic driver is in my opinion a very good idea, but > > we can't break the existing support. Porting the rcar-du driver to the > > component model is needed. > > is this in someones plans already? Because I'm still having trouble > understanding parts of the rockchip driver sometimes, so feel in no way > qualified to try to get this tackled in some reasonable timeframe :-) currently my idea is to simply do something like the following for this :-) static const struct of_device_id rcar_du_of_table[] = { { .compatible = "renesas,du-r8a7779" }, { .compatible = "renesas,du-r8a7790" }, { .compatible = "renesas,du-r8a7791" }, { } }; static int __init vga_encoder_init(void) { struct device_node *np; /* * Play nice with rcar-du that is having its own implementation * of the adv7123 binding implementation and is not yet * converted to using components. */ np = of_find_matching_node(NULL, rcar_du_of_table); if (np) { of_node_put(np); return 0; } return platform_driver_register(&vga_encoder_driver); } slightly similar to what some iommus do > > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > > > --- > > > > > > drivers/gpu/drm/i2c/Kconfig | 6 + > > > drivers/gpu/drm/i2c/Makefile | 2 + > > > drivers/gpu/drm/i2c/vga-simple.c | 325 > > > ++++++++++++++++++++++++++++++++++++ > > > > drivers/gpu/drm/i2c/ feels wrong for a platform device. > > yep, i2c probably being the wrong place was one of the caveats in the cover > letter and I was hoping some more seasoned drm people could suggest where > this should live after all. > > As your comments further down suggest that we might introduce some different > components, simply congregate them in a "components" subdir or something > splitting them more? so does a "components" subdir look reasonable? > > > 3 files changed, 333 insertions(+) > > > create mode 100644 drivers/gpu/drm/i2c/vga-simple.c > > > > > > diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig > > > index 22c7ed6..319f2cb 100644 > > > --- a/drivers/gpu/drm/i2c/Kconfig > > > +++ b/drivers/gpu/drm/i2c/Kconfig > > > @@ -31,4 +31,10 @@ config DRM_I2C_NXP_TDA998X > > > > > > help > > > > > > Support for NXP Semiconductors TDA998X HDMI encoders. > > > > > > +config DRM_VGA_SIMPLE > > > + tristate "Generic simple vga encoder" > > > + help > > > + Support for vga encoder chips without any special settings > > > + and at most a power-control gpio. > > > + > > > > > > endmenu > > > > > > diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile > > > index 2c72eb5..858961f 100644 > > > --- a/drivers/gpu/drm/i2c/Makefile > > > +++ b/drivers/gpu/drm/i2c/Makefile > > > @@ -10,3 +10,5 @@ obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o > > > > > > tda998x-y := tda998x_drv.o > > > obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o > > > > > > + > > > +obj-$(CONFIG_DRM_VGA_SIMPLE) += vga-simple.o > > > diff --git a/drivers/gpu/drm/i2c/vga-simple.c > > > b/drivers/gpu/drm/i2c/vga-simple.c new file mode 100644 > > > index 0000000..bb9d19c > > > --- /dev/null > > > +++ b/drivers/gpu/drm/i2c/vga-simple.c > > > @@ -0,0 +1,325 @@ > > > +/* > > > + * Simple vga encoder driver > > > + * > > > + * Copyright (C) 2014 Heiko Stuebner <heiko@sntech.de> > > > + * > > > + * This software is licensed under the terms of the GNU General Public > > > + * License version 2, as published by the Free Software Foundation, and > > > + * may be copied, distributed, and modified under those terms. > > > + * > > > + * This program is distributed in the hope that it will be useful, > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > > + * GNU General Public License for more details. > > > + */ > > > + > > > +#include <linux/module.h> > > > +#include <linux/component.h> > > > +#include <linux/gpio.h> > > > +#include <linux/gpio/consumer.h> > > > +#include <linux/i2c.h> > > > +#include <linux/regulator/consumer.h> > > > +#include <drm/drm_of.h> > > > +#include <drm/drmP.h> > > > +#include <drm/drm_crtc.h> > > > +#include <drm/drm_crtc_helper.h> > > > +#include <drm/drm_edid.h> > > > + > > > +#define connector_to_vga_simple(x) container_of(x, struct vga_simple, > > > connector) +#define encoder_to_vga_simple(x) container_of(x, struct > > > vga_simple, encoder) + > > > +struct vga_simple { > > > + struct drm_connector connector; > > > + struct drm_encoder encoder; > > > + > > > + struct device *dev; > > > + struct i2c_adapter *ddc; > > > + > > > + struct regulator *vaa_reg; > > > + struct gpio_desc *enable_gpio; > > > + > > > + struct mutex enable_lock; > > > + bool enabled; > > > +}; > > > + > > > +/* > > > + * Connector functions > > > + */ > > > + > > > +enum drm_connector_status vga_simple_detect(struct drm_connector > > > *connector, + bool force) > > > +{ > > > + struct vga_simple *vga = connector_to_vga_simple(connector); > > > + > > > + if (!vga->ddc) > > > + return connector_status_unknown; > > > + > > > + if (drm_probe_ddc(vga->ddc)) > > > + return connector_status_connected; > > > + > > > + return connector_status_disconnected; > > > +} > > > + > > > +void vga_simple_connector_destroy(struct drm_connector *connector) > > > +{ > > > + drm_connector_unregister(connector); > > > + drm_connector_cleanup(connector); > > > +} > > > + > > > +struct drm_connector_funcs vga_simple_connector_funcs = { > > > + .dpms = drm_helper_connector_dpms, > > > + .fill_modes = drm_helper_probe_single_connector_modes, > > > + .detect = vga_simple_detect, > > > + .destroy = vga_simple_connector_destroy, > > > +}; > > > + > > > +/* > > > + * Connector helper functions > > > + */ > > > + > > > +static int vga_simple_connector_get_modes(struct drm_connector > > > *connector) > > > +{ > > > + struct vga_simple *vga = connector_to_vga_simple(connector); > > > + struct edid *edid; > > > + int ret = 0; > > > + > > > + if (!vga->ddc) > > > + return 0; > > > + > > > + edid = drm_get_edid(connector, vga->ddc); > > > + if (edid) { > > > + drm_mode_connector_update_edid_property(connector, edid); > > > + ret = drm_add_edid_modes(connector, edid); > > > + kfree(edid); > > > + } > > > + > > > + return ret; > > > +} > > > + > > > +static int vga_simple_connector_mode_valid(struct drm_connector > > > *connector, + struct drm_display_mode *mode) > > > +{ > > > + return MODE_OK; > > > +} > > > + > > > +static struct drm_encoder > > > +*vga_simple_connector_best_encoder(struct drm_connector *connector) > > > +{ > > > + struct vga_simple *vga = connector_to_vga_simple(connector); > > > + > > > + return &vga->encoder; > > > +} > > > + > > > +static struct drm_connector_helper_funcs > > > vga_simple_connector_helper_funcs > > > = { + .get_modes = vga_simple_connector_get_modes, > > > + .best_encoder = vga_simple_connector_best_encoder, > > > + .mode_valid = vga_simple_connector_mode_valid, > > > +}; > > > + > > > +/* > > > + * Encoder functions > > > + */ > > > + > > > +static void vga_simple_encoder_destroy(struct drm_encoder *encoder) > > > +{ > > > + drm_encoder_cleanup(encoder); > > > +} > > > + > > > +static const struct drm_encoder_funcs vga_simple_encoder_funcs = { > > > + .destroy = vga_simple_encoder_destroy, > > > +}; > > > + > > > +/* > > > + * Encoder helper functions > > > + */ > > > + > > > +static void vga_simple_encoder_dpms(struct drm_encoder *encoder, int > > > mode) > > > +{ > > > + struct vga_simple *vga = encoder_to_vga_simple(encoder); > > > + > > > + mutex_lock(&vga->enable_lock); > > > + > > > + switch (mode) { > > > + case DRM_MODE_DPMS_ON: > > > + if (vga->enabled) > > > + goto out; > > > + > > > + if (!IS_ERR(vga->vaa_reg)) > > > + regulator_enable(vga->vaa_reg); > > > + > > > + if (vga->enable_gpio) > > > + gpiod_set_value(vga->enable_gpio, 1); > > > + > > > + vga->enabled = true; > > > + break; > > > + case DRM_MODE_DPMS_STANDBY: > > > + case DRM_MODE_DPMS_SUSPEND: > > > + case DRM_MODE_DPMS_OFF: > > > + if (!vga->enabled) > > > + goto out; > > > + > > > + if (vga->enable_gpio) > > > + gpiod_set_value(vga->enable_gpio, 0); > > > + > > > + if (!IS_ERR(vga->vaa_reg)) > > > + regulator_enable(vga->vaa_reg); > > > + > > > + vga->enabled = false; > > > + break; > > > + default: > > > + break; > > > + } > > > + > > > +out: > > > + mutex_unlock(&vga->enable_lock); > > > +} > > > + > > > +static bool vga_simple_mode_fixup(struct drm_encoder *encoder, > > > + const struct drm_display_mode *mode, > > > + struct drm_display_mode *adjusted_mode) > > > +{ > > > + return true; > > > +} > > > + > > > +static void vga_simple_encoder_prepare(struct drm_encoder *encoder) > > > +{ > > > +} > > > + > > > +static void vga_simple_encoder_mode_set(struct drm_encoder *encoder, > > > + struct drm_display_mode *mode, > > > + struct drm_display_mode *adjusted_mode) > > > +{ > > > +} > > > + > > > +static void vga_simple_encoder_commit(struct drm_encoder *encoder) > > > +{ > > > + vga_simple_encoder_dpms(encoder, DRM_MODE_DPMS_ON); > > > +} > > > + > > > +static void vga_simple_encoder_disable(struct drm_encoder *encoder) > > > +{ > > > + vga_simple_encoder_dpms(encoder, DRM_MODE_DPMS_OFF); > > > +} > > > + > > > +static const struct drm_encoder_helper_funcs > > > vga_simple_encoder_helper_funcs = { > > > + .dpms = vga_simple_encoder_dpms, > > > + .mode_fixup = vga_simple_mode_fixup, > > > + .prepare = vga_simple_encoder_prepare, > > > + .mode_set = vga_simple_encoder_mode_set, > > > + .commit = vga_simple_encoder_commit, > > > + .disable = vga_simple_encoder_disable, > > > > this is interesting. Some users of this encoder (I'm thinking about > > rcar-du, for which I've just posted the patches) are already ported to > > atomic updates, while others are not. How can we support both ? > > Wild guess, support both in such generic components and let bind decide > which to use ... is there some sort of drm_is_atomic() against the master > device. my hope here is that till I am finished with my part, the atomic conversion will have turned up "magically" :-) > > > +}; > > > + > > > +/* > > > + * Component helper functions > > > + */ > > > + > > > +static int vga_simple_bind(struct device *dev, struct device *master, > > > + void *data) > > > +{ > > > + struct platform_device *pdev = to_platform_device(dev); > > > + struct drm_device *drm = data; > > > + > > > + struct device_node *ddc_node, *np = pdev->dev.of_node; > > > + struct vga_simple *vga; > > > + int ret; > > > + > > > + if (!np) > > > + return -ENODEV; > > > + > > > + vga = devm_kzalloc(dev, sizeof(*vga), GFP_KERNEL); > > > + if (!vga) > > > + return -ENOMEM; > > > + > > > + vga->dev = dev; > > > + dev_set_drvdata(dev, vga); > > > + mutex_init(&vga->enable_lock); > > > + > > > + vga->enable_gpio = devm_gpiod_get_optional(dev, "enable", > > > + GPIOD_OUT_LOW); > > > + if (IS_ERR(vga->enable_gpio)) { > > > + ret = PTR_ERR(vga->enable_gpio); > > > + dev_err(dev, "failed to request GPIO: %d\n", ret); > > > + return ret; > > > + } > > > + > > > + vga->enabled = false; > > > + > > > + ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); > > > + if (ddc_node) { > > > + vga->ddc = of_find_i2c_adapter_by_node(ddc_node); > > > + of_node_put(ddc_node); > > > + if (!vga->ddc) { > > > + dev_dbg(vga->dev, "failed to read ddc node\n"); > > > + return -EPROBE_DEFER; > > > + } > > > + } else { > > > + dev_dbg(vga->dev, "no ddc property found\n"); > > > + } > > > + > > > + vga->vaa_reg = devm_regulator_get_optional(dev, "vaa"); > > > + > > > + vga->encoder.possible_crtcs = drm_of_find_possible_crtcs(drm, np); > > > + vga->encoder.of_node = np; > > > + vga->connector.polled = DRM_CONNECTOR_POLL_CONNECT | > > > + DRM_CONNECTOR_POLL_DISCONNECT; > > > + > > > + drm_encoder_helper_add(&vga->encoder, > > &vga_simple_encoder_helper_funcs); > > > > + drm_encoder_init(drm, &vga->encoder, &vga_simple_encoder_funcs, > > > + DRM_MODE_ENCODER_DAC); > > > + > > > + drm_connector_helper_add(&vga->connector, > > > + &vga_simple_connector_helper_funcs); > > > > I really dislike this, this is an encoder driver, not a connector driver. > > It shouldn't be responsible for creating the connector. For all it knows, > > there might not even be a VGA connector connected to the encoder, VGA > > signals could be sent to a chained encoder. That might be a bit > > far-fetched in the VGA case, but in the generic case encoder drivers > > shouldn't create connectors. > > ok, so you suggest to have this split. Taking into account Rob's comment > probably a "simple-encoder" and a "ddc-connector" for connectors using > system i2c busses? And then connect everything via ports. having them separate actually looks quite interesting, so definitly no argument from me against doing a "vga-encoder" and "vga-connector" driver using the already established bindings. Heiko
diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig index 22c7ed6..319f2cb 100644 --- a/drivers/gpu/drm/i2c/Kconfig +++ b/drivers/gpu/drm/i2c/Kconfig @@ -31,4 +31,10 @@ config DRM_I2C_NXP_TDA998X help Support for NXP Semiconductors TDA998X HDMI encoders. +config DRM_VGA_SIMPLE + tristate "Generic simple vga encoder" + help + Support for vga encoder chips without any special settings + and at most a power-control gpio. + endmenu diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile index 2c72eb5..858961f 100644 --- a/drivers/gpu/drm/i2c/Makefile +++ b/drivers/gpu/drm/i2c/Makefile @@ -10,3 +10,5 @@ obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o tda998x-y := tda998x_drv.o obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o + +obj-$(CONFIG_DRM_VGA_SIMPLE) += vga-simple.o diff --git a/drivers/gpu/drm/i2c/vga-simple.c b/drivers/gpu/drm/i2c/vga-simple.c new file mode 100644 index 0000000..bb9d19c --- /dev/null +++ b/drivers/gpu/drm/i2c/vga-simple.c @@ -0,0 +1,325 @@ +/* + * Simple vga encoder driver + * + * Copyright (C) 2014 Heiko Stuebner <heiko@sntech.de> + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <linux/module.h> +#include <linux/component.h> +#include <linux/gpio.h> +#include <linux/gpio/consumer.h> +#include <linux/i2c.h> +#include <linux/regulator/consumer.h> +#include <drm/drm_of.h> +#include <drm/drmP.h> +#include <drm/drm_crtc.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_edid.h> + +#define connector_to_vga_simple(x) container_of(x, struct vga_simple, connector) +#define encoder_to_vga_simple(x) container_of(x, struct vga_simple, encoder) + +struct vga_simple { + struct drm_connector connector; + struct drm_encoder encoder; + + struct device *dev; + struct i2c_adapter *ddc; + + struct regulator *vaa_reg; + struct gpio_desc *enable_gpio; + + struct mutex enable_lock; + bool enabled; +}; + +/* + * Connector functions + */ + +enum drm_connector_status vga_simple_detect(struct drm_connector *connector, + bool force) +{ + struct vga_simple *vga = connector_to_vga_simple(connector); + + if (!vga->ddc) + return connector_status_unknown; + + if (drm_probe_ddc(vga->ddc)) + return connector_status_connected; + + return connector_status_disconnected; +} + +void vga_simple_connector_destroy(struct drm_connector *connector) +{ + drm_connector_unregister(connector); + drm_connector_cleanup(connector); +} + +struct drm_connector_funcs vga_simple_connector_funcs = { + .dpms = drm_helper_connector_dpms, + .fill_modes = drm_helper_probe_single_connector_modes, + .detect = vga_simple_detect, + .destroy = vga_simple_connector_destroy, +}; + +/* + * Connector helper functions + */ + +static int vga_simple_connector_get_modes(struct drm_connector *connector) +{ + struct vga_simple *vga = connector_to_vga_simple(connector); + struct edid *edid; + int ret = 0; + + if (!vga->ddc) + return 0; + + edid = drm_get_edid(connector, vga->ddc); + if (edid) { + drm_mode_connector_update_edid_property(connector, edid); + ret = drm_add_edid_modes(connector, edid); + kfree(edid); + } + + return ret; +} + +static int vga_simple_connector_mode_valid(struct drm_connector *connector, + struct drm_display_mode *mode) +{ + return MODE_OK; +} + +static struct drm_encoder +*vga_simple_connector_best_encoder(struct drm_connector *connector) +{ + struct vga_simple *vga = connector_to_vga_simple(connector); + + return &vga->encoder; +} + +static struct drm_connector_helper_funcs vga_simple_connector_helper_funcs = { + .get_modes = vga_simple_connector_get_modes, + .best_encoder = vga_simple_connector_best_encoder, + .mode_valid = vga_simple_connector_mode_valid, +}; + +/* + * Encoder functions + */ + +static void vga_simple_encoder_destroy(struct drm_encoder *encoder) +{ + drm_encoder_cleanup(encoder); +} + +static const struct drm_encoder_funcs vga_simple_encoder_funcs = { + .destroy = vga_simple_encoder_destroy, +}; + +/* + * Encoder helper functions + */ + +static void vga_simple_encoder_dpms(struct drm_encoder *encoder, int mode) +{ + struct vga_simple *vga = encoder_to_vga_simple(encoder); + + mutex_lock(&vga->enable_lock); + + switch (mode) { + case DRM_MODE_DPMS_ON: + if (vga->enabled) + goto out; + + if (!IS_ERR(vga->vaa_reg)) + regulator_enable(vga->vaa_reg); + + if (vga->enable_gpio) + gpiod_set_value(vga->enable_gpio, 1); + + vga->enabled = true; + break; + case DRM_MODE_DPMS_STANDBY: + case DRM_MODE_DPMS_SUSPEND: + case DRM_MODE_DPMS_OFF: + if (!vga->enabled) + goto out; + + if (vga->enable_gpio) + gpiod_set_value(vga->enable_gpio, 0); + + if (!IS_ERR(vga->vaa_reg)) + regulator_enable(vga->vaa_reg); + + vga->enabled = false; + break; + default: + break; + } + +out: + mutex_unlock(&vga->enable_lock); +} + +static bool vga_simple_mode_fixup(struct drm_encoder *encoder, + const struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) +{ + return true; +} + +static void vga_simple_encoder_prepare(struct drm_encoder *encoder) +{ +} + +static void vga_simple_encoder_mode_set(struct drm_encoder *encoder, + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) +{ +} + +static void vga_simple_encoder_commit(struct drm_encoder *encoder) +{ + vga_simple_encoder_dpms(encoder, DRM_MODE_DPMS_ON); +} + +static void vga_simple_encoder_disable(struct drm_encoder *encoder) +{ + vga_simple_encoder_dpms(encoder, DRM_MODE_DPMS_OFF); +} + +static const struct drm_encoder_helper_funcs vga_simple_encoder_helper_funcs = { + .dpms = vga_simple_encoder_dpms, + .mode_fixup = vga_simple_mode_fixup, + .prepare = vga_simple_encoder_prepare, + .mode_set = vga_simple_encoder_mode_set, + .commit = vga_simple_encoder_commit, + .disable = vga_simple_encoder_disable, +}; + +/* + * Component helper functions + */ + +static int vga_simple_bind(struct device *dev, struct device *master, + void *data) +{ + struct platform_device *pdev = to_platform_device(dev); + struct drm_device *drm = data; + + struct device_node *ddc_node, *np = pdev->dev.of_node; + struct vga_simple *vga; + int ret; + + if (!np) + return -ENODEV; + + vga = devm_kzalloc(dev, sizeof(*vga), GFP_KERNEL); + if (!vga) + return -ENOMEM; + + vga->dev = dev; + dev_set_drvdata(dev, vga); + mutex_init(&vga->enable_lock); + + vga->enable_gpio = devm_gpiod_get_optional(dev, "enable", + GPIOD_OUT_LOW); + if (IS_ERR(vga->enable_gpio)) { + ret = PTR_ERR(vga->enable_gpio); + dev_err(dev, "failed to request GPIO: %d\n", ret); + return ret; + } + + vga->enabled = false; + + ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); + if (ddc_node) { + vga->ddc = of_find_i2c_adapter_by_node(ddc_node); + of_node_put(ddc_node); + if (!vga->ddc) { + dev_dbg(vga->dev, "failed to read ddc node\n"); + return -EPROBE_DEFER; + } + } else { + dev_dbg(vga->dev, "no ddc property found\n"); + } + + vga->vaa_reg = devm_regulator_get_optional(dev, "vaa"); + + vga->encoder.possible_crtcs = drm_of_find_possible_crtcs(drm, np); + vga->encoder.of_node = np; + vga->connector.polled = DRM_CONNECTOR_POLL_CONNECT | + DRM_CONNECTOR_POLL_DISCONNECT; + + drm_encoder_helper_add(&vga->encoder, &vga_simple_encoder_helper_funcs); + drm_encoder_init(drm, &vga->encoder, &vga_simple_encoder_funcs, + DRM_MODE_ENCODER_DAC); + + drm_connector_helper_add(&vga->connector, + &vga_simple_connector_helper_funcs); + drm_connector_init(drm, &vga->connector, &vga_simple_connector_funcs, + DRM_MODE_CONNECTOR_VGA); + + drm_mode_connector_attach_encoder(&vga->connector, &vga->encoder); + + return 0; +} + +static void vga_simple_unbind(struct device *dev, struct device *master, + void *data) +{ + struct vga_simple *vga = dev_get_drvdata(dev); + + vga->connector.funcs->destroy(&vga->connector); + vga->encoder.funcs->destroy(&vga->encoder); +} + +static const struct component_ops vga_simple_ops = { + .bind = vga_simple_bind, + .unbind = vga_simple_unbind, +}; + +static int vga_simple_probe(struct platform_device *pdev) +{ + return component_add(&pdev->dev, &vga_simple_ops); +} + +static int vga_simple_remove(struct platform_device *pdev) +{ + component_del(&pdev->dev, &vga_simple_ops); + + return 0; +} + +static const struct of_device_id vga_simple_ids[] = { + { .compatible = "adi,adv7123", }, + {}, +}; +MODULE_DEVICE_TABLE(of, vga_simple_ids); + +static struct platform_driver vga_simple_driver = { + .probe = vga_simple_probe, + .remove = vga_simple_remove, + .driver = { + .name = "vga-simple", + .of_match_table = vga_simple_ids, + }, +}; +module_platform_driver(vga_simple_driver); + +MODULE_AUTHOR("Heiko Stuebner <heiko@sntech.de>"); +MODULE_DESCRIPTION("Simple vga converter"); +MODULE_LICENSE("GPL");
There exist simple vga encoders without any type of management interface and just maybe a simple gpio for turning it on or off. Examples for these are the Analog Devices ADV7123, Chipsea CS7123 or Micronas SDA7123. Add a generic encoder driver for those that can be used by drm drivers using the component framework. Signed-off-by: Heiko Stuebner <heiko@sntech.de> --- drivers/gpu/drm/i2c/Kconfig | 6 + drivers/gpu/drm/i2c/Makefile | 2 + drivers/gpu/drm/i2c/vga-simple.c | 325 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 333 insertions(+) create mode 100644 drivers/gpu/drm/i2c/vga-simple.c