Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754814Ab3JCSXb (ORCPT ); Thu, 3 Oct 2013 14:23:31 -0400 Received: from mail-wg0-f49.google.com ([74.125.82.49]:37642 "EHLO mail-wg0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754349Ab3JCSX2 (ORCPT ); Thu, 3 Oct 2013 14:23:28 -0400 MIME-Version: 1.0 In-Reply-To: References: <1380670860-17621-1-git-send-email-seanpaul@chromium.org> <1380670860-17621-4-git-send-email-seanpaul@chromium.org> Date: Fri, 4 Oct 2013 03:23:26 +0900 X-Google-Sender-Auth: AxGm_K8_ZwGYByEnipH9oSNtdSk Message-ID: Subject: Re: [PATCH 3/5] drm/bridge: Add PTN3460 bridge driver From: Inki Dae To: Sean Paul Cc: "devicetree@vger.kernel.org" , "linux-samsung-soc@vger.kernel.org" , "linux-doc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , DRI mailing list , "linux-arm-kernel@lists.infradead.org" Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 24503 Lines: 640 2013/10/4 Sean Paul : > On Thu, Oct 3, 2013 at 1:39 PM, Inki Dae wrote: >> 2013/10/3 Sean Paul : >>> On Thu, Oct 3, 2013 at 9:55 AM, Inki Dae wrote: >>>> Hi, thank you for your contribution and the below is my short comments, >>>> >>>> 2013/10/2 Sean Paul : >>>>> This patch adds a drm_bridge driver for the PTN3460 DisplayPort to LVDS >>>>> bridge chip. >>>>> >>>>> Signed-off-by: Sean Paul >>>>> --- >>>>> .../devicetree/bindings/drm/bridge/ptn3460.txt | 27 ++ >>>>> drivers/gpu/drm/Kconfig | 2 + >>>>> drivers/gpu/drm/Makefile | 1 + >>>>> drivers/gpu/drm/bridge/Kconfig | 4 + >>>>> drivers/gpu/drm/bridge/Makefile | 3 + >>>>> drivers/gpu/drm/bridge/ptn3460.c | 349 +++++++++++++++++++++ >>>>> include/drm/bridge/ptn3460.h | 36 +++ >>>>> 7 files changed, 422 insertions(+) >>>>> create mode 100644 Documentation/devicetree/bindings/drm/bridge/ptn3460.txt >>>>> create mode 100644 drivers/gpu/drm/bridge/Kconfig >>>>> create mode 100644 drivers/gpu/drm/bridge/Makefile >>>>> create mode 100644 drivers/gpu/drm/bridge/ptn3460.c >>>>> create mode 100644 include/drm/bridge/ptn3460.h >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/drm/bridge/ptn3460.txt b/Documentation/devicetree/bindings/drm/bridge/ptn3460.txt >>>>> new file mode 100644 >>>>> index 0000000..c1cd329 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/drm/bridge/ptn3460.txt >>>>> @@ -0,0 +1,27 @@ >>>>> +ptn3460-bridge bindings >>>>> + >>>>> +Required properties: >>>>> + - compatible: "nxp,ptn3460" >>>>> + - reg: i2c address of the bridge >>>>> + - powerdown-gpio: OF device-tree gpio specification >>>> >>>> Can a regulator be used instead of gpio in other board case? >>>> >>> >>> No, not to my knowledge. >>> >> >> Hm.. plz check it out again. the gpio pin is specific to board, and >> the the gpio be used as power source trigger could be replaced with a >> regulator according to board design. So you should consider all >> possibilities even though there are no other cases yet: other board >> could use a regulator instead. >> >>> >>>>> + - reset-gpio: OF device-tree gpio specification >>>>> + - edid-emulation: The EDID emulation entry to use >>>>> + +-------+------------+------------------+ >>>>> + | Value | Resolution | Description | >>>>> + | 0 | 1024x768 | NXP Generic | >>>>> + | 1 | 1920x1080 | NXP Generic | >>>>> + | 2 | 1920x1080 | NXP Generic | >>>>> + | 3 | 1600x900 | Samsung LTM200KT | >>>>> + | 4 | 1920x1080 | Samsung LTM230HT | >>>>> + | 5 | 1366x768 | NXP Generic | >>>>> + | 6 | 1600x900 | ChiMei M215HGE | >>>>> + +-------+------------+------------------+ >>>>> + >>>>> +Example: >>>>> + ptn3460-bridge@20 { >>>>> + compatible = "nxp,ptn3460"; >>>>> + reg = <0x20>; >>>>> + powerdown-gpio = <&gpy2 5 1 0 0>; >>>>> + reset-gpio = <&gpx1 5 1 0 0>; >>>>> + edid-emulation = <5>; >>>>> + }; >>>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >>>>> index 955555d..cd7bfb3 100644 >>>>> --- a/drivers/gpu/drm/Kconfig >>>>> +++ b/drivers/gpu/drm/Kconfig >>>>> @@ -236,3 +236,5 @@ source "drivers/gpu/drm/tilcdc/Kconfig" >>>>> source "drivers/gpu/drm/qxl/Kconfig" >>>>> >>>>> source "drivers/gpu/drm/msm/Kconfig" >>>>> + >>>>> +source "drivers/gpu/drm/bridge/Kconfig" >>>>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile >>>>> index f089adf..9234253 100644 >>>>> --- a/drivers/gpu/drm/Makefile >>>>> +++ b/drivers/gpu/drm/Makefile >>>>> @@ -56,3 +56,4 @@ obj-$(CONFIG_DRM_TILCDC) += tilcdc/ >>>>> obj-$(CONFIG_DRM_QXL) += qxl/ >>>>> obj-$(CONFIG_DRM_MSM) += msm/ >>>>> obj-y += i2c/ >>>>> +obj-y += bridge/ >>>>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig >>>>> new file mode 100644 >>>>> index 0000000..f8db069 >>>>> --- /dev/null >>>>> +++ b/drivers/gpu/drm/bridge/Kconfig >>>>> @@ -0,0 +1,4 @@ >>>>> +config DRM_PTN3460 >>>>> + tristate "PTN3460 DP/LVDS bridge" >>>>> + depends on DRM && I2C >>>>> + ---help--- >>>>> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile >>>>> new file mode 100644 >>>>> index 0000000..b4733e1 >>>>> --- /dev/null >>>>> +++ b/drivers/gpu/drm/bridge/Makefile >>>>> @@ -0,0 +1,3 @@ >>>>> +ccflags-y := -Iinclude/drm >>>>> + >>>>> +obj-$(CONFIG_DRM_PTN3460) += ptn3460.o >>>>> diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c >>>>> new file mode 100644 >>>>> index 0000000..a9e5c1a >>>>> --- /dev/null >>>>> +++ b/drivers/gpu/drm/bridge/ptn3460.c >>>>> @@ -0,0 +1,349 @@ >>>>> +/* >>>>> + * NXP PTN3460 DP/LVDS bridge driver >>>>> + * >>>>> + * Copyright (C) 2013 Google, Inc. >>>>> + * >>>>> + * 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 >>>>> +#include >>>>> +#include >>>>> +#include >>>>> +#include >>>>> +#include >>>>> + >>>>> +#include "drmP.h" >>>>> +#include "drm_edid.h" >>>>> +#include "drm_crtc.h" >>>>> +#include "drm_crtc_helper.h" >>>>> + >>>>> +#include "bridge/ptn3460.h" >>>>> + >>>>> +#define PTN3460_EDID_ADDR 0x0 >>>>> +#define PTN3460_EDID_EMULATION_ADDR 0x84 >>>>> +#define PTN3460_EDID_ENABLE_EMULATION 0 >>>>> +#define PTN3460_EDID_EMULATION_SELECTION 1 >>>>> +#define PTN3460_EDID_SRAM_LOAD_ADDR 0x85 >>>>> + >>>>> +struct ptn3460_bridge { >>>>> + struct drm_connector connector; >>>>> + struct i2c_client *client; >>>>> + struct drm_encoder *encoder; >>>>> + struct drm_bridge *bridge; >>>>> + struct edid *edid; >>>>> + int gpio_pd_n; >>>>> + int gpio_rst_n; >>>>> + u32 edid_emulation; >>>>> + bool enabled; >>>>> +}; >>>>> + >>>>> +static int ptn3460_read_bytes(struct ptn3460_bridge *ptn_bridge, char addr, >>>>> + u8 *buf, int len) >>>>> +{ >>>>> + int ret; >>>>> + >>>>> + ret = i2c_master_send(ptn_bridge->client, &addr, 1); >>>>> + if (ret <= 0) { >>>>> + DRM_ERROR("Failed to send i2c command, ret=%d\n", ret); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + ret = i2c_master_recv(ptn_bridge->client, buf, len); >>>>> + if (ret <= 0) { >>>>> + DRM_ERROR("Failed to recv i2c data, ret=%d\n", ret); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int ptn3460_write_byte(struct ptn3460_bridge *ptn_bridge, char addr, >>>>> + char val) >>>>> +{ >>>>> + int ret; >>>>> + char buf[2]; >>>>> + >>>>> + buf[0] = addr; >>>>> + buf[1] = val; >>>>> + >>>>> + ret = i2c_master_send(ptn_bridge->client, buf, ARRAY_SIZE(buf)); >>>>> + if (ret <= 0) { >>>>> + DRM_ERROR("Failed to send i2c command, ret=%d\n", ret); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int ptn3460_select_edid(struct ptn3460_bridge *ptn_bridge) >>>>> +{ >>>>> + int ret; >>>>> + char val; >>>>> + >>>>> + /* Load the selected edid into SRAM (accessed at PTN3460_EDID_ADDR) */ >>>>> + ret = ptn3460_write_byte(ptn_bridge, PTN3460_EDID_SRAM_LOAD_ADDR, >>>>> + ptn_bridge->edid_emulation); >>>>> + if (ret) { >>>>> + DRM_ERROR("Failed to transfer edid to sram, ret=%d\n", ret); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + /* Enable EDID emulation and select the desired EDID */ >>>>> + val = 1 << PTN3460_EDID_ENABLE_EMULATION | >>>>> + ptn_bridge->edid_emulation << PTN3460_EDID_EMULATION_SELECTION; >>>>> + >>>>> + ret = ptn3460_write_byte(ptn_bridge, PTN3460_EDID_EMULATION_ADDR, val); >>>>> + if (ret) { >>>>> + DRM_ERROR("Failed to write edid value, ret=%d\n", ret); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static void ptn3460_pre_enable(struct drm_bridge *bridge) >>>>> +{ >>>>> + struct ptn3460_bridge *ptn_bridge = bridge->driver_private; >>>>> + int ret; >>>>> + >>>>> + if (ptn_bridge->enabled) >>>>> + return; >>>>> + >>>>> + if (gpio_is_valid(ptn_bridge->gpio_pd_n)) >>>>> + gpio_set_value(ptn_bridge->gpio_pd_n, 1); >>>> >>>> Ditto. >>>> >>>>> + >>>>> + if (gpio_is_valid(ptn_bridge->gpio_rst_n)) { >>>>> + gpio_set_value(ptn_bridge->gpio_rst_n, 0); >>>>> + udelay(10); >>>>> + gpio_set_value(ptn_bridge->gpio_rst_n, 1); >>>>> + } >>>>> + >>>>> + /* >>>>> + * There's a bug in the PTN chip where it falsely asserts hotplug before >>>>> + * it is fully functional. We're forced to wait for the maximum start up >>>>> + * time specified in the chip's datasheet to make sure we're really up. >>>>> + */ >>>>> + msleep(90); >>>>> + >>>>> + ret = ptn3460_select_edid(ptn_bridge); >>>>> + if (ret) >>>>> + DRM_ERROR("Select edid failed ret=%d\n", ret); >>>>> + >>>>> + ptn_bridge->enabled = true; >>>>> +} >>>>> + >>>>> +static void ptn3460_enable(struct drm_bridge *bridge) >>>>> +{ >>>>> +} >>>>> + >>>>> +static void ptn3460_disable(struct drm_bridge *bridge) >>>>> +{ >>>>> + struct ptn3460_bridge *ptn_bridge = bridge->driver_private; >>>>> + >>>>> + if (!ptn_bridge->enabled) >>>>> + return; >>>>> + >>>>> + ptn_bridge->enabled = false; >>>>> + >>>>> + if (gpio_is_valid(ptn_bridge->gpio_rst_n)) >>>>> + gpio_set_value(ptn_bridge->gpio_rst_n, 1); >>>>> + >>>>> + if (gpio_is_valid(ptn_bridge->gpio_pd_n)) >>>>> + gpio_set_value(ptn_bridge->gpio_pd_n, 0); >>>> >>>> Ditto. >>>> >>>>> +} >>>>> + >>>>> +static void ptn3460_post_disable(struct drm_bridge *bridge) >>>>> +{ >>>>> +} >>>>> + >>>>> +void ptn3460_bridge_destroy(struct drm_bridge *bridge) >>>>> +{ >>>>> + struct ptn3460_bridge *ptn_bridge = bridge->driver_private; >>>>> + >>>>> + drm_bridge_cleanup(bridge); >>>>> + if (gpio_is_valid(ptn_bridge->gpio_pd_n)) >>>>> + gpio_free(ptn_bridge->gpio_pd_n); >>>> >>>> Ditto. >>>> >>>>> + if (gpio_is_valid(ptn_bridge->gpio_rst_n)) >>>>> + gpio_free(ptn_bridge->gpio_rst_n); >>>>> + /* Nothing else to free, we've got devm allocated memory */ >>>>> +} >>>>> + >>>>> +struct drm_bridge_funcs ptn3460_bridge_funcs = { >>>>> + .pre_enable = ptn3460_pre_enable, >>>>> + .enable = ptn3460_enable, >>>>> + .disable = ptn3460_disable, >>>>> + .post_disable = ptn3460_post_disable, >>>>> + .destroy = ptn3460_bridge_destroy, >>>>> +}; >>>>> + >>>>> +int ptn3460_get_modes(struct drm_connector *connector) >>>>> +{ >>>>> + struct ptn3460_bridge *ptn_bridge; >>>>> + u8 *edid; >>>>> + int ret, num_modes; >>>>> + bool power_off; >>>>> + >>>>> + ptn_bridge = container_of(connector, struct ptn3460_bridge, connector); >>>>> + >>>>> + if (ptn_bridge->edid) >>>>> + return drm_add_edid_modes(connector, ptn_bridge->edid); >>>>> + >>>>> + power_off = !ptn_bridge->enabled; >>>>> + ptn3460_pre_enable(ptn_bridge->bridge); >>>>> + >>>>> + edid = kmalloc(EDID_LENGTH, GFP_KERNEL); >>>>> + if (!edid) { >>>>> + DRM_ERROR("Failed to allocate edid\n"); >>>>> + return 0; >>>>> + } >>>>> + >>>>> + ret = ptn3460_read_bytes(ptn_bridge, PTN3460_EDID_ADDR, edid, >>>>> + EDID_LENGTH); >>>>> + if (ret) { >>>>> + kfree(edid); >>>>> + num_modes = 0; >>>>> + goto out; >>>>> + } >>>>> + >>>>> + ptn_bridge->edid = (struct edid *)edid; >>>>> + drm_mode_connector_update_edid_property(connector, ptn_bridge->edid); >>>>> + >>>>> + num_modes = drm_add_edid_modes(connector, ptn_bridge->edid); >>>>> + >>>>> +out: >>>>> + if (power_off) >>>>> + ptn3460_disable(ptn_bridge->bridge); >>>>> + >>>>> + return num_modes; >>>>> +} >>>>> + >>>>> +static int ptn3460_mode_valid(struct drm_connector *connector, >>>>> + struct drm_display_mode *mode) >>>>> +{ >>>>> + return MODE_OK; >>>>> +} >>>>> + >>>>> +struct drm_encoder *ptn3460_best_encoder(struct drm_connector *connector) >>>>> +{ >>>>> + struct ptn3460_bridge *ptn_bridge; >>>>> + >>>>> + ptn_bridge = container_of(connector, struct ptn3460_bridge, connector); >>>>> + >>>>> + return ptn_bridge->encoder; >>>>> +} >>>>> + >>>>> +struct drm_connector_helper_funcs ptn3460_connector_helper_funcs = { >>>>> + .get_modes = ptn3460_get_modes, >>>>> + .mode_valid = ptn3460_mode_valid, >>>>> + .best_encoder = ptn3460_best_encoder, >>>>> +}; >>>>> + >>>>> +enum drm_connector_status ptn3460_detect(struct drm_connector *connector, >>>>> + bool force) >>>>> +{ >>>>> + return connector_status_connected; >>>>> +} >>>>> + >>>>> +void ptn3460_connector_destroy(struct drm_connector *connector) >>>>> +{ >>>>> + drm_connector_cleanup(connector); >>>>> +} >>>>> + >>>>> +struct drm_connector_funcs ptn3460_connector_funcs = { >>>>> + .dpms = drm_helper_connector_dpms, >>>>> + .fill_modes = drm_helper_probe_single_connector_modes, >>>>> + .detect = ptn3460_detect, >>>>> + .destroy = ptn3460_connector_destroy, >>>>> +}; >>>> >>>> Why do you try to add a new connector here? We already have the >>>> connector for LCD, and also provides some callbacks for it. For this, >>>> please see exynos_drm_display_ops of exynos_drm_fimd driver, and you >>>> can add new callbacks to there such as init callback for bridge device >>>> initialization if needed. >>>> >>> >>> We add a new connector for 2 reasons: >>> >>> 1) We need to override the drm detect() callback to always return true >>> since the DP driver will presumably return its hotplug status which >>> will always be low when the ptn chip is turned off. >>> 2) We want the ability to control the result of get_modes(). >>> >>> I've got a patch set almost ready to tear the display ops out of fimd >>> and put them in the DP driver. >> >> Really? if so, that is ideal something we want and we should go. But >> isn't the DP driver placed in drivers/video/exynos? How did you take >> care of that? > > git mv :) > :) >> Actually, for this, we planned to use CDF(Common Display >> Framework) if the framework is merged to mainline somehow. >> > > Right. I think CDF will end up being a series of improvements to drm, > as opposed to its own framework (at least this was the conclusion I > came to after speaking with Laurent at the plumbers conference). I > don't think it makes sense to have the DP driver outside of drm. The > HDMI driver is already inside drm, the DP driver should be too. See the below, Application -------------------------------------------------------------- v4l2 drm kms hdmi driver hdmi driver ------------------------------------------------------------- hdmi hw HDMI is a controller can be controlled by user application, and for this some frameworks such as v4l2 and drm kms interfaces are used. But DP, MIPI-DSI, LVDS, and so on aren't controlled by user application. These are just display bus between scanout devices(hdmi, fimd) and lcd panel. So the above your example doesn't make sense. > Regardless, this conversation is only tangentially related to this > patch and can probably be deferred. > > Sean > >>> The display ops are better suited >>> there, since it's the actual encoder/connector. I hope to get that >>> posted this week. >>> >> >> I will look forward to that posting. :) >> >>> >>>>> + >>>>> +int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder, >>>>> + struct i2c_client *client, struct device_node *node) >>>>> +{ >>>>> + int ret; >>>>> + struct drm_bridge *bridge; >>>>> + struct ptn3460_bridge *ptn_bridge; >>>>> + >>>>> + bridge = devm_kzalloc(dev->dev, sizeof(*bridge), GFP_KERNEL); >>>>> + if (!bridge) { >>>>> + DRM_ERROR("Failed to allocate drm bridge\n"); >>>>> + return -ENOMEM; >>>>> + } >>>>> + >>>>> + ptn_bridge = devm_kzalloc(dev->dev, sizeof(*ptn_bridge), GFP_KERNEL); >>>>> + if (!ptn_bridge) { >>>>> + DRM_ERROR("Failed to allocate ptn bridge\n"); >>>>> + return -ENOMEM; >>>>> + } >>>>> + >>>>> + ptn_bridge->client = client; >>>>> + ptn_bridge->encoder = encoder; >>>>> + ptn_bridge->bridge = bridge; >>>>> + ptn_bridge->gpio_pd_n = of_get_named_gpio(node, "powerdown-gpio", 0); >>>> >>>> Also, if a regulator is used instead? >>>> >>>>> + if (gpio_is_valid(ptn_bridge->gpio_pd_n)) { >>>>> + ret = gpio_request_one(ptn_bridge->gpio_pd_n, >>>>> + GPIOF_OUT_INIT_HIGH, "PTN3460_PD_N"); >>>>> + if (ret) { >>>>> + DRM_ERROR("Request powerdown-gpio failed (%d)\n", ret); >>>>> + return ret; >>>>> + } >>>>> + } >>>>> + >>>>> + ptn_bridge->gpio_rst_n = of_get_named_gpio(node, "reset-gpio", 0); >>>>> + if (gpio_is_valid(ptn_bridge->gpio_rst_n)) { >>>>> + /* >>>>> + * Request the reset pin low to avoid the bridge being >>>>> + * initialized prematurely >>>>> + */ >>>>> + ret = gpio_request_one(ptn_bridge->gpio_rst_n, >>>>> + GPIOF_OUT_INIT_LOW, "PTN3460_RST_N"); >>>>> + if (ret) { >>>>> + DRM_ERROR("Request reset-gpio failed (%d)\n", ret); >>>>> + gpio_free(ptn_bridge->gpio_pd_n); >>>>> + return ret; >>>>> + } >>>>> + } >>>>> + >>>>> + ret = of_property_read_u32(node, "edid-emulation", >>>>> + &ptn_bridge->edid_emulation); >>>>> + if (ret) { >>>>> + DRM_ERROR("Can't read edid emulation value\n"); >>>>> + goto err; >>>>> + } >>>>> + >>>>> + ret = drm_bridge_init(dev, bridge, &ptn3460_bridge_funcs); >>>>> + if (ret) { >>>>> + DRM_ERROR("Failed to initialize bridge with drm\n"); >>>>> + goto err; >>>>> + } >>>>> + >>>>> + bridge->driver_private = ptn_bridge; >>>>> + encoder->bridge = bridge; >>>>> + >>>>> + ret = drm_connector_init(dev, &ptn_bridge->connector, >>>>> + &ptn3460_connector_funcs, DRM_MODE_CONNECTOR_LVDS); >>>> >>>> So it seems that here's not right place to call drm_connector_init function. >>>> >>>> Display pipeline path could be one of, >>>> Display Controller Display bus >>>> --------------------------------------------------------------------------- >>>> FIMD---------------------LVDS--------------------LCD, >>>> or >>>> FIMD----------------------eDP---------------------LCD, >>>> or >>>> FIMD------------------MIPI-DSI------------------LCD, >>>> or >>>> FIMD-------------------------------------------------LCD >>>> >>>> And also in case using image enhancement chip, >>>> mDNIe-------------FIMD-LITE between Display Controller and Display >>>> bus. >>>> >>>> So, wouldn't the right place below FIMD driver? :) >>>> >>> >>> Well, this driver should be considered outside of exynos context since >>> it could be used by any drm driver. >>> >>> In the exynos context, the right place to implement it would be in the >>> dp driver, actually. However, the exynos driver has a level of >>> abstraction on top of the crtcs/encoders such that we need to >>> initialize it in the exynos_drm_core. The patchset I mentioned above >>> should help move things in a direction where fimd/mixer implement >>> drm_crtc directly and hdmi/dp implement drm_encoder/drm_connector >>> directly. In that world, DP would initialize the ptn driver. >>> >>>> >>>>> + if (ret) { >>>>> + DRM_ERROR("Failed to initialize connector with drm\n"); >>>>> + goto err; >>>>> + } >>>>> + drm_connector_helper_add(&ptn_bridge->connector, >>>>> + &ptn3460_connector_helper_funcs); >>>>> + drm_sysfs_connector_add(&ptn_bridge->connector); >>>>> + drm_mode_connector_attach_encoder(&ptn_bridge->connector, encoder); >>>>> + >>>>> + return 0; >>>>> + >>>>> +err: >>>>> + if (gpio_is_valid(ptn_bridge->gpio_pd_n)) >>>>> + gpio_free(ptn_bridge->gpio_pd_n); >>>>> + if (gpio_is_valid(ptn_bridge->gpio_rst_n)) >>>>> + gpio_free(ptn_bridge->gpio_rst_n); >>>>> + return ret; >>>>> +} >>>>> diff --git a/include/drm/bridge/ptn3460.h b/include/drm/bridge/ptn3460.h >>>>> new file mode 100644 >>>>> index 0000000..157ffa1 >>>>> --- /dev/null >>>>> +++ b/include/drm/bridge/ptn3460.h >>>>> @@ -0,0 +1,36 @@ >>>>> +/* >>>>> + * Copyright (C) 2013 Google, Inc. >>>>> + * >>>>> + * 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. >>>>> + */ >>>>> + >>>>> +#ifndef _DRM_BRIDGE_PTN3460_H_ >>>>> +#define _DRM_BRIDGE_PTN3460_H_ >>>>> + >>>>> +struct drm_device; >>>>> +struct drm_encoder; >>>>> +struct i2c_client; >>>>> +struct device_node; >>>>> + >>>>> +#ifdef CONFIG_DRM_PTN3460 >>>>> + >>>>> +int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder, >>>>> + struct i2c_client *client, struct device_node *node); >>>>> +#else >>>>> + >>>>> +int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder, >>>>> + struct i2c_client *client, struct device_node *node) >>>>> +{ >>>>> + return 0; >>>>> +} >>>>> + >>>>> +#endif >>>>> + >>>>> +#endif >>>>> -- >>>>> 1.8.4 >>>>> >>>>> >>>>> _______________________________________________ >>>>> linux-arm-kernel mailing list >>>>> linux-arm-kernel@lists.infradead.org >>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >>> _______________________________________________ >>> dri-devel mailing list >>> dri-devel@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/