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@xxxxxxxxx>
> ---
> 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@xxxxxxxxx>
> + *
> + * 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@xxxxxxxxx>");
> +MODULE_DESCRIPTION("Simple vga converter");
> +MODULE_LICENSE("GPL");
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
