On Tuesday, April 22, 2014 7:39 AM, Ajay Kumar wrote:

This patch adds a simple driver to handle all the LCD and LED powerup/down routines needed to support eDP/eDP-LVDS panels supported on exynos boards.

The LCD and LED units are usually powered up via regulators, and almost on all boards, we will have a BL_EN pin to enable/ disable the backlight. Sometimes, we can have LCD_EN switches as well. The routines in this driver can be used to control panel power sequence on such boards.

Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com

Changes since V1: Added routine for post_disable, removed few unwanted headers. Refactored a lot of code.

.../devicetree/bindings/panel/exynos-dp-panel.txt | 45 ++++ drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-exynos-dp.c | 251 ++++++++++++++++++++ 4 files changed, 306 insertions(+) create mode 100644 Documentation/devicetree/bindings/panel/exynos-dp-panel.txt create mode 100644 drivers/gpu/drm/panel/panel-exynos-dp.c

diff --git a/Documentation/devicetree/bindings/panel/exynos-dp-panel.txt b/Documentation/devicetree/bindings/panel/exynos-dp-panel.txt new file mode 100644 index 0000000..3fbca54 --- /dev/null +++ b/Documentation/devicetree/bindings/panel/exynos-dp-panel.txt @@ -0,0 +1,45 @@ +exynos_DP_panel/DP_to_LVDS_panel

Please fix it as below.

+Exynos DP panel/DP to LVDS panel

[....]

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 4ec874d..ea9d5ac 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -30,4 +30,13 @@ config DRM_PANEL_S6E8AA0 select DRM_MIPI_DSI select VIDEOMODE_HELPERS

+config DRM_PANEL_EXYNOS_DP

• tristate "support for DP panels"

It looks very general. Please fix it as below.

+ tristate "support for Exynos DP panels"

Best regards, Jingoo Han

• depends on OF && DRM_PANEL && DRM_EXYNOS_DP
• help

•  DRM panel driver for DP panels and LVDS connected via DP bridges
  

•  that need at most a regulator for LCD unit, a regulator for LED unit
  

•  and/or enable GPIOs for LCD or LED units. Delay values can also be
  

•  specified to support powerup and powerdown process.
  

[.....]

On Tue, Apr 22, 2014 at 04:09:13AM +0530, Ajay Kumar wrote:

Register exynos_dp_panel before the list of exynos crtcs and connectors are probed.

This is needed because exynos_dp_panel should be registered to the drm_panel list via panel-exynos-dp probe, i.e much before exynos_dp_bind calls of_drm_find_panel().

Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com

Changes since V1: Added platform_driver_unregister(&exynos_dp_panel_driver) to exynos_drm_platform_remove as per Jingoo Han's correction

drivers/gpu/drm/exynos/exynos_drm_drv.c | 15 +++++++++++++++ drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 + 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index 1d653f8..2db7f67 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -530,12 +530,23 @@ static int exynos_drm_platform_probe(struct platform_device *pdev) goto err_unregister_ipp_drv; #endif

+#ifdef CONFIG_DRM_PANEL_EXYNOS_DP

• ret = platform_driver_register(&exynos_dp_panel_driver);
• if (ret < 0)

• goto err_unregister_dp_panel;
  

+#endif

No, this is not how you're supposed to use DRM panel drivers. The idea is that you write a standalone driver for a given panel.

What you do here has a number of problems. For one it's a driver that's tightly coupled to Exynos SoCs. But if I have a different SoC that uses the same panel I want to be able to use the same driver, and not have to rewrite the driver for my SoC.

Another problem is that you're assuming here that the driver is built in and it will break if you try to build either Exynos DRM or the panel driver as a module. This is perhaps nothing you care about right now, but eventually people will want to ship a single kernel that can run on a number of SoCs. But if we keep adding things like this, that kernel will keep growing in size until it no longer fits in any kind of memory.

Thierry

On Tue, Apr 22, 2014 at 08:33:23PM +0530, Ajay kumar wrote:

Hi Thierry,

On Tue, Apr 22, 2014 at 2:03 PM, Thierry Reding thierry.reding@gmail.com wrote:

...

On Tue, Apr 22, 2014 at 04:09:13AM +0530, Ajay Kumar wrote:

...

Register exynos_dp_panel before the list of exynos crtcs and connectors are probed.

This is needed because exynos_dp_panel should be registered to the drm_panel list via panel-exynos-dp probe, i.e much before exynos_dp_bind calls of_drm_find_panel().

Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com

Changes since V1: Added platform_driver_unregister(&exynos_dp_panel_driver) to exynos_drm_platform_remove as per Jingoo Han's correction

drivers/gpu/drm/exynos/exynos_drm_drv.c | 15 +++++++++++++++ drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 + 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index 1d653f8..2db7f67 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -530,12 +530,23 @@ static int exynos_drm_platform_probe(struct platform_device *pdev) goto err_unregister_ipp_drv; #endif

+#ifdef CONFIG_DRM_PANEL_EXYNOS_DP

• ret = platform_driver_register(&exynos_dp_panel_driver);
  

• if (ret < 0)
  

•         goto err_unregister_dp_panel;
  

+#endif

No, this is not how you're supposed to use DRM panel drivers. The idea is that you write a standalone driver for a given panel.

What you do here has a number of problems. For one it's a driver that's tightly coupled to Exynos SoCs. But if I have a different SoC that uses the same panel I want to be able to use the same driver, and not have to rewrite the driver for my SoC.

Another problem is that you're assuming here that the driver is built in and it will break if you try to build either Exynos DRM or the panel driver as a module. This is perhaps nothing you care about right now, but eventually people will want to ship a single kernel that can run on a number of SoCs. But if we keep adding things like this, that kernel will keep growing in size until it no longer fits in any kind of memory.

Thierry

I completely agree with you in this!

Yes, this is not acceptable, but I want to know an "acceptable" workaround for the situation below: I register the driver using module_init(). And, exynos_drm gets probed much before the panel driver probe happens. So, the panel driver hasn't probed yet, but exynos_dp via exynos_drm tries to call "of_drm_find_panel" which always returns NULL.

That's a situation that your driver needs to be able to deal with. The driver registration order doesn't matter one bit. It may happen to work most of the time, but as soon as one of the resources that your panel driver needs isn't there when the panel is probed, then it won't be registered and of_drm_find_panel() will still return NULL.

Usually the right thing to do in that case would be to return (and propagate) -EPROBE_DEFER so that your driver's probe is deferred and retried when other drivers have been probed. That way it should eventually get a non-NULL panel.

Thierry

On Tue, Aug 19, 2014 at 09:02:39PM -0700, Stéphane Marchesin wrote:

On Tue, Apr 22, 2014 at 8:26 AM, Thierry Reding thierry.reding@gmail.com wrote:

...

On Tue, Apr 22, 2014 at 08:33:23PM +0530, Ajay kumar wrote:

...

Hi Thierry,

On Tue, Apr 22, 2014 at 2:03 PM, Thierry Reding thierry.reding@gmail.com wrote:

...

On Tue, Apr 22, 2014 at 04:09:13AM +0530, Ajay Kumar wrote:

...

Register exynos_dp_panel before the list of exynos crtcs and connectors are probed.

This is needed because exynos_dp_panel should be registered to the drm_panel list via panel-exynos-dp probe, i.e much before exynos_dp_bind calls of_drm_find_panel().

Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com

Changes since V1: Added platform_driver_unregister(&exynos_dp_panel_driver) to exynos_drm_platform_remove as per Jingoo Han's correction

drivers/gpu/drm/exynos/exynos_drm_drv.c | 15 +++++++++++++++ drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 + 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index 1d653f8..2db7f67 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -530,12 +530,23 @@ static int exynos_drm_platform_probe(struct platform_device *pdev) goto err_unregister_ipp_drv; #endif

+#ifdef CONFIG_DRM_PANEL_EXYNOS_DP

• ret = platform_driver_register(&exynos_dp_panel_driver);
  

• if (ret < 0)
  

•         goto err_unregister_dp_panel;
  

+#endif

No, this is not how you're supposed to use DRM panel drivers. The idea is that you write a standalone driver for a given panel.

What you do here has a number of problems. For one it's a driver that's tightly coupled to Exynos SoCs. But if I have a different SoC that uses the same panel I want to be able to use the same driver, and not have to rewrite the driver for my SoC.

Another problem is that you're assuming here that the driver is built in and it will break if you try to build either Exynos DRM or the panel driver as a module. This is perhaps nothing you care about right now, but eventually people will want to ship a single kernel that can run on a number of SoCs. But if we keep adding things like this, that kernel will keep growing in size until it no longer fits in any kind of memory.

Thierry

I completely agree with you in this!

Yes, this is not acceptable, but I want to know an "acceptable" workaround for the situation below: I register the driver using module_init(). And, exynos_drm gets probed much before the panel driver probe happens. So, the panel driver hasn't probed yet, but exynos_dp via exynos_drm tries to call "of_drm_find_panel" which always returns NULL.

That's a situation that your driver needs to be able to deal with. The driver registration order doesn't matter one bit. It may happen to work most of the time, but as soon as one of the resources that your panel driver needs isn't there when the panel is probed, then it won't be registered and of_drm_find_panel() will still return NULL.

Usually the right thing to do in that case would be to return (and propagate) -EPROBE_DEFER so that your driver's probe is deferred and retried when other drivers have been probed. That way it should eventually get a non-NULL panel.

So I just gave this (drm_panel + probe deferring) a shot on exynos, and correctly reacting to -EPROBE_DEFER postpones DP initialization by approximately 1.5 second. Is there a good way to handle that? As it stands, this isn't usable.

How much is 1.5 seconds compared to the overall boot time of the device? What exactly is causing this 1.5 second delay?

This really is a fundamental issue with deferred probing and the issue has come up several times in the past. A couple of possible solutions have been proposed, with the latest being here[0] I think. That ended in a bit of a debacle, unfortunately, but on of the outcomes was that a lot of the ordering problems could be fixed by using phandle references to track dependencies. I'm not aware of anyone working on that right now, presumably because everyone is busy getting features merged rather than optimizing boot speed.

Thierry

[0]: https://lkml.org/lkml/2014/5/12/452

On Thu, Aug 21, 2014 at 1:55 PM, Stéphane Marchesin stephane.marchesin@gmail.com wrote:

On Thu, Aug 21, 2014 at 12:36 AM, Thierry Reding thierry.reding@gmail.com wrote:

...

On Tue, Aug 19, 2014 at 09:02:39PM -0700, Stéphane Marchesin wrote:

...

On Tue, Apr 22, 2014 at 8:26 AM, Thierry Reding thierry.reding@gmail.com wrote:

...

On Tue, Apr 22, 2014 at 08:33:23PM +0530, Ajay kumar wrote:

...

Hi Thierry,

On Tue, Apr 22, 2014 at 2:03 PM, Thierry Reding thierry.reding@gmail.com wrote:

...

On Tue, Apr 22, 2014 at 04:09:13AM +0530, Ajay Kumar wrote: > Register exynos_dp_panel before the list of exynos crtcs and > connectors are probed. > > This is needed because exynos_dp_panel should be registered to > the drm_panel list via panel-exynos-dp probe, i.e much before > exynos_dp_bind calls of_drm_find_panel(). > > Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com > --- > Changes since V1: > Added platform_driver_unregister(&exynos_dp_panel_driver) to > exynos_drm_platform_remove as per Jingoo Han's correction > > drivers/gpu/drm/exynos/exynos_drm_drv.c | 15 +++++++++++++++ > drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 + > 2 files changed, 16 insertions(+) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c > index 1d653f8..2db7f67 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c > @@ -530,12 +530,23 @@ static int exynos_drm_platform_probe(struct platform_device *pdev) > goto err_unregister_ipp_drv; > #endif > > +#ifdef CONFIG_DRM_PANEL_EXYNOS_DP > + ret = platform_driver_register(&exynos_dp_panel_driver); > + if (ret < 0) > + goto err_unregister_dp_panel; > +#endif

No, this is not how you're supposed to use DRM panel drivers. The idea is that you write a standalone driver for a given panel.

What you do here has a number of problems. For one it's a driver that's tightly coupled to Exynos SoCs. But if I have a different SoC that uses the same panel I want to be able to use the same driver, and not have to rewrite the driver for my SoC.

Another problem is that you're assuming here that the driver is built in and it will break if you try to build either Exynos DRM or the panel driver as a module. This is perhaps nothing you care about right now, but eventually people will want to ship a single kernel that can run on a number of SoCs. But if we keep adding things like this, that kernel will keep growing in size until it no longer fits in any kind of memory.

Thierry

I completely agree with you in this!

Yes, this is not acceptable, but I want to know an "acceptable" workaround for the situation below: I register the driver using module_init(). And, exynos_drm gets probed much before the panel driver probe happens. So, the panel driver hasn't probed yet, but exynos_dp via exynos_drm tries to call "of_drm_find_panel" which always returns NULL.

That's a situation that your driver needs to be able to deal with. The driver registration order doesn't matter one bit. It may happen to work most of the time, but as soon as one of the resources that your panel driver needs isn't there when the panel is probed, then it won't be registered and of_drm_find_panel() will still return NULL.

Usually the right thing to do in that case would be to return (and propagate) -EPROBE_DEFER so that your driver's probe is deferred and retried when other drivers have been probed. That way it should eventually get a non-NULL panel.

So I just gave this (drm_panel + probe deferring) a shot on exynos, and correctly reacting to -EPROBE_DEFER postpones DP initialization by approximately 1.5 second. Is there a good way to handle that? As it stands, this isn't usable.

How much is 1.5 seconds compared to the overall boot time of the device?

1.5s is 15-20% of my boot time (if you count the boot time from firmware start to login prompt, otherwise it's more). Note that on other platforms, we've seen this take as much as 5 or 6s, but for the exynos case it is "only" 1.5s.

...

What exactly is causing this 1.5 second delay?

A regulator isn't ready, and then drm_panel returns defer. Then the whole drm driver init is deferred.

...

This really is a fundamental issue with deferred probing and the issue has come up several times in the past. A couple of possible solutions have been proposed, with the latest being here[0] I think. That ended in a bit of a debacle, unfortunately, but on of the outcomes was that a lot of the ordering problems could be fixed by using phandle references to track dependencies. I'm not aware of anyone working on that right now, presumably because everyone is busy getting features merged rather than optimizing boot speed.

Yes, I don't believe boot time ordering will ever happen upstream, but then the current implementation with EPROBE_DEFER isn't usable either. Any ideas? ATM it seems like the only way out is to just write my own dt format for the panel and ignore drm_panel.

Something like this: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3...

With this way also, we would expect the regulator to come up early(before drm).

Ajay

Adding Boris and Ludovic since this topic came up in a different thread as well.

On Thu, Aug 21, 2014 at 01:25:07AM -0700, Stéphane Marchesin wrote:

On Thu, Aug 21, 2014 at 12:36 AM, Thierry Reding thierry.reding@gmail.com wrote:

...

On Tue, Aug 19, 2014 at 09:02:39PM -0700, Stéphane Marchesin wrote:

...

On Tue, Apr 22, 2014 at 8:26 AM, Thierry Reding thierry.reding@gmail.com wrote:

...

On Tue, Apr 22, 2014 at 08:33:23PM +0530, Ajay kumar wrote:

...

Hi Thierry,

On Tue, Apr 22, 2014 at 2:03 PM, Thierry Reding thierry.reding@gmail.com wrote:

...

On Tue, Apr 22, 2014 at 04:09:13AM +0530, Ajay Kumar wrote: > Register exynos_dp_panel before the list of exynos crtcs and > connectors are probed. > > This is needed because exynos_dp_panel should be registered to > the drm_panel list via panel-exynos-dp probe, i.e much before > exynos_dp_bind calls of_drm_find_panel(). > > Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com > --- > Changes since V1: > Added platform_driver_unregister(&exynos_dp_panel_driver) to > exynos_drm_platform_remove as per Jingoo Han's correction > > drivers/gpu/drm/exynos/exynos_drm_drv.c | 15 +++++++++++++++ > drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 + > 2 files changed, 16 insertions(+) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c > index 1d653f8..2db7f67 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c > @@ -530,12 +530,23 @@ static int exynos_drm_platform_probe(struct platform_device *pdev) > goto err_unregister_ipp_drv; > #endif > > +#ifdef CONFIG_DRM_PANEL_EXYNOS_DP > + ret = platform_driver_register(&exynos_dp_panel_driver); > + if (ret < 0) > + goto err_unregister_dp_panel; > +#endif

No, this is not how you're supposed to use DRM panel drivers. The idea is that you write a standalone driver for a given panel.

What you do here has a number of problems. For one it's a driver that's tightly coupled to Exynos SoCs. But if I have a different SoC that uses the same panel I want to be able to use the same driver, and not have to rewrite the driver for my SoC.

Another problem is that you're assuming here that the driver is built in and it will break if you try to build either Exynos DRM or the panel driver as a module. This is perhaps nothing you care about right now, but eventually people will want to ship a single kernel that can run on a number of SoCs. But if we keep adding things like this, that kernel will keep growing in size until it no longer fits in any kind of memory.

Thierry

I completely agree with you in this!

Yes, this is not acceptable, but I want to know an "acceptable" workaround for the situation below: I register the driver using module_init(). And, exynos_drm gets probed much before the panel driver probe happens. So, the panel driver hasn't probed yet, but exynos_dp via exynos_drm tries to call "of_drm_find_panel" which always returns NULL.

That's a situation that your driver needs to be able to deal with. The driver registration order doesn't matter one bit. It may happen to work most of the time, but as soon as one of the resources that your panel driver needs isn't there when the panel is probed, then it won't be registered and of_drm_find_panel() will still return NULL.

Usually the right thing to do in that case would be to return (and propagate) -EPROBE_DEFER so that your driver's probe is deferred and retried when other drivers have been probed. That way it should eventually get a non-NULL panel.

So I just gave this (drm_panel + probe deferring) a shot on exynos, and correctly reacting to -EPROBE_DEFER postpones DP initialization by approximately 1.5 second. Is there a good way to handle that? As it stands, this isn't usable.

How much is 1.5 seconds compared to the overall boot time of the device?

1.5s is 15-20% of my boot time (if you count the boot time from firmware start to login prompt, otherwise it's more). Note that on other platforms, we've seen this take as much as 5 or 6s, but for the exynos case it is "only" 1.5s.

...

What exactly is causing this 1.5 second delay?

A regulator isn't ready, and then drm_panel returns defer. Then the whole drm driver init is deferred.

That is the correct way to do this currently. It's unfortunate that it causes such long delays.

...

This really is a fundamental issue with deferred probing and the issue has come up several times in the past. A couple of possible solutions have been proposed, with the latest being here[0] I think. That ended in a bit of a debacle, unfortunately, but on of the outcomes was that a lot of the ordering problems could be fixed by using phandle references to track dependencies. I'm not aware of anyone working on that right now, presumably because everyone is busy getting features merged rather than optimizing boot speed.

Yes, I don't believe boot time ordering will ever happen upstream,

Maybe explicit boot time ordering won't happen upstream, but I don't think anyone would object if you posted patches that decrease boot time significantly, provided that they're still a generic solution. Tracking dependencies parse from a device tree sounds like an option that would be acceptable.

but then the current implementation with EPROBE_DEFER isn't usable either.

"Isn't usable" is pretty strong. It does get you a working panel, right? It's perhaps not optimized, but it gives you something that's guaranteed to work (unless you have an actual bug somewhere), and something that works in the general case, for everyone.

Any ideas? ATM it seems like the only way out is to just write my own dt format for the panel and ignore drm_panel.

Well, since probe order is currently dependent on device instantiation order, one possibility would be to rearrange the DT nodes in a way that dependencies are listed first. So in your case I'd expect the regulator provider to be listed first, followed by the panel and finally the "DRM" device. Providing none of them trigger deferred probing themselves they should probe in exactly that order.

Of course there are no guarantees that the probing order will stay that way forever and you will only be solving this problem for one device at a time rather than in general (which is the primary reason for deferred probing in the first place).

One other possibility I guess would be not to rely on deferred probing but to enable DRM/KMS to hotplug panels. Theoretically this should be possible already but it comes at a cost. If I'm not mistaken that's what Boris implemented for the Atmel HLCDC driver. However if the panel isn't available at DRM driver ->load() time, then it relies on the hotplugging mechanism to detect the panel later on, which can itself introduce more delays. I suspect this could be improved in various ways, but one of the more complicated issues is that it causes problems when you want to use the framebuffer console. The issue is that the fbdev helper code is set up very early and if no outputs with valid modes are found it will default to a 1024x768 resolution for the framebuffer console. At the same time it will prevent any outputs that are hotplugged later on to increase the resolution again. This works fine for monitors which usually support a number of different modes (1024x768 being standard enough that probably every monitor in existence supports it). But for panels this won't work, since they usually support only a single mode.

There are a number of ways that this could be solved I guess. One would be to defer fbdev creation itself until all outputs have been detected. This could prove difficult because it may not always be possible to determine which outputs to wait for. Another solution would be to allow the framebuffer console to be resized, but I've been told in the past that it's impractical if not impossible.

Of course if you don't need a framebuffer console (you could use kmscon or none at all), then things should work for you if you simply consider the panel as hotpluggable monitor.

Thierry

On Tue, Apr 22, 2014 at 04:09:14AM +0530, Ajay Kumar wrote: [...]

diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c

[...]

@@ -1299,6 +1308,15 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)

INIT_WORK(&dp->hotplug_work, exynos_dp_hotplug);

• panel_node = of_find_compatible_node(NULL, NULL,

• 				"samsung,exynos-dp-panel");
  

No, please don't do this. It will break as soon as you have two panels of the same type in one system.

Also the compatible value of a panel should describe the specific panel in use (e.g. "samsung,s6e8aa0"), so you shouldn't rely on "magic" like this. Use a phandle to find the panel's struct device_node.

[...]

diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.h b/drivers/gpu/drm/exynos/exynos_dp_core.h index 56fa43e..9dc7991 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.h +++ b/drivers/gpu/drm/exynos/exynos_dp_core.h @@ -148,6 +148,7 @@ struct exynos_dp_device { struct drm_device *drm_dev; struct drm_connector connector; struct drm_encoder *encoder;

• struct drm_panel *drm_panel;

I don't think you need the drm_ prefix here.

Thierry

On Tue, Apr 22, 2014 at 04:09:16AM +0530, Ajay Kumar wrote: [...]

diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c

[...]

@@ -1012,9 +1013,16 @@ static int exynos_dp_create_connector(struct exynos_drm_display *display, dp->encoder = encoder;

/* Pre-empt DP connector creation if there's a bridge */

• ret = exynos_drm_attach_lcd_bridge(dp->drm_dev, encoder);
• if (ret)

• ret = exynos_drm_attach_lcd_bridge(dp->drm_dev, encoder, dp->drm_panel);
• if (ret) {

• /*
  

•  * Also set "dp->drm_panel = NULL" so that we don't end up
  

•  * controlling panel power both in exynos_dp and bridge
  

•  * DPMS routines.
  

•  */
  

• dp->drm_panel = NULL;
  

  return 0;
• }

This kind of hack should set off an alarm that you're doing something wrong. I'm not sure integration of bridges and panels has been thought about or discussed a lot, but this doesn't look right to me at all.

Thierry

On Tue, Apr 22, 2014 at 07:34:03AM -0400, Rob Clark wrote:

So what about, rather than adding drm_panel support to each bridge individually, we introduce a drm_panel_bridge (with a form of chaining).. ie:

struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; struct drm_bridge *bridge; /* optional */ };

static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pb = to_panel_bridge(bridge); if (pb->bridge) pb->bridge->funcs->enable(pb->bridge); pb->panel->funcs->enable(pb->panel); }

... and so on.

If you don't need a real bridge, just create one of these w/ pb->bridge==NULL, otherwise create it as a wrapper for your real bridge.

Yeah I think that's how I'd have implemented some generic abstraction for drivers using the crtc helpers. This allows you to keep bridge drivers, panel drivers and anything else you might have in your driver to feed the pixel stream to those 2 guys separate. And it also allows you to set it all up in different ways, e.g. using device tree metadata, or acpi or some other table hardcoded in a video rom somewhere.

Imo we also should have something similar to chain up drm_bridge devices. tbh I'm not terribly happy about how the current integration with the crtc modeset helpers is done ... -Daniel

BR, -R

On Mon, Apr 21, 2014 at 6:39 PM, Ajay Kumar ajaykumar.rs@samsung.com wrote:

...

attach ptn3460 connector to drm_panel and support drm_panel routines, if a valid drm_panel object is passed to ptn3460_init.

Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com

Changes since V1: Address few coding style comments from Jingoo Han

drivers/gpu/drm/bridge/Kconfig | 1 + drivers/gpu/drm/bridge/ptn3460.c | 20 +++++++++++++++++++- drivers/gpu/drm/exynos/exynos_dp_core.c | 16 ++++++++++++---- include/drm/bridge/ptn3460.h | 6 ++++-- 4 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 884923f..3bc6845 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -2,4 +2,5 @@ config DRM_PTN3460 tristate "PTN3460 DP/LVDS bridge" depends on DRM select DRM_KMS_HELPER

•   select DRM_PANEL
    ---help---
  

diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c index f1d2afc..3920202 100644 --- a/drivers/gpu/drm/bridge/ptn3460.c +++ b/drivers/gpu/drm/bridge/ptn3460.c @@ -19,6 +19,7 @@ #include <linux/i2c.h> #include <linux/gpio.h> #include <linux/delay.h> +#include <drm/drm_panel.h>

#include "drmP.h" #include "drm_edid.h" @@ -38,6 +39,7 @@ struct ptn3460_bridge { struct i2c_client *client; struct drm_encoder *encoder; struct drm_bridge *bridge;

•   struct drm_panel *panel;
    struct edid *edid;
    int gpio_pd_n;
    int gpio_rst_n;
  

@@ -126,6 +128,8 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge) gpio_set_value(ptn_bridge->gpio_rst_n, 1); }

•   drm_panel_pre_enable(ptn_bridge->panel);
  

•   /*
     * 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
  

@@ -142,6 +146,10 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)

static void ptn3460_enable(struct drm_bridge *bridge) {

•   struct ptn3460_bridge *ptn_bridge = bridge->driver_private;
  

•   if (ptn_bridge->enabled)
  

•           drm_panel_enable(ptn_bridge->panel);
  

}

static void ptn3460_disable(struct drm_bridge *bridge) @@ -153,6 +161,9 @@ static void ptn3460_disable(struct drm_bridge *bridge)

    ptn_bridge->enabled = false;

•   drm_panel_disable(ptn_bridge->panel);
  

•   drm_panel_post_disable(ptn_bridge->panel);
  

•   if (gpio_is_valid(ptn_bridge->gpio_rst_n))
            gpio_set_value(ptn_bridge->gpio_rst_n, 1);
  
  

@@ -198,6 +209,7 @@ int ptn3460_get_modes(struct drm_connector *connector)

    power_off = !ptn_bridge->enabled;
    ptn3460_pre_enable(ptn_bridge->bridge);

•   ptn3460_enable(ptn_bridge->bridge);
  
    edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
    if (!edid) {
  

@@ -265,7 +277,8 @@ struct drm_connector_funcs ptn3460_connector_funcs = { };

int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,

•           struct i2c_client *client, struct device_node *node)
  

•           struct i2c_client *client, struct device_node *node,
  

•           struct drm_panel *panel)
  

{ int ret; struct drm_bridge *bridge; @@ -324,6 +337,11 @@ int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder, goto err; }

•   if (panel) {
  

•           ptn_bridge->panel = panel;
  

•           drm_panel_attach(ptn_bridge->panel, &ptn_bridge->connector);
  

•   }
  

•   bridge->driver_private = ptn_bridge;
    encoder->bridge = bridge;
    ptn_bridge->connector.polled = DRM_CONNECTOR_POLL_HPD;
  

diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c index dbc5ccc..4853f31 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c @@ -989,13 +989,14 @@ static bool find_bridge(const char *compat, struct bridge_init *bridge)

/* returns the number of bridges attached */ static int exynos_drm_attach_lcd_bridge(struct drm_device *dev,

•           struct drm_encoder *encoder)
  

•           struct drm_encoder *encoder, struct drm_panel *panel)
  

{ struct bridge_init bridge; int ret;

    if (find_bridge("nxp,ptn3460", &bridge)) {

•           ret = ptn3460_init(dev, encoder, bridge.client, bridge.node);
  

•           ret = ptn3460_init(dev, encoder, bridge.client, bridge.node,
  

•                           panel);
            if (!ret)
                    return 1;
    }
  

@@ -1012,9 +1013,16 @@ static int exynos_dp_create_connector(struct exynos_drm_display *display, dp->encoder = encoder;

    /* Pre-empt DP connector creation if there's a bridge */

•   ret = exynos_drm_attach_lcd_bridge(dp->drm_dev, encoder);
  

•   if (ret)
  

•   ret = exynos_drm_attach_lcd_bridge(dp->drm_dev, encoder, dp->drm_panel);
  

•   if (ret) {
  

•           /*
  

•            * Also set "dp->drm_panel = NULL" so that we don't end up
  

•            * controlling panel power both in exynos_dp and bridge
  

•            * DPMS routines.
  

•            */
  

•           dp->drm_panel = NULL;
            return 0;
  

•   }
  
    connector->polled = DRM_CONNECTOR_POLL_HPD;
  
  

diff --git a/include/drm/bridge/ptn3460.h b/include/drm/bridge/ptn3460.h index ff62344..570cebb 100644 --- a/include/drm/bridge/ptn3460.h +++ b/include/drm/bridge/ptn3460.h @@ -18,16 +18,18 @@ struct drm_device; struct drm_encoder; struct i2c_client; struct device_node; +struct drm_panel;

#if defined(CONFIG_DRM_PTN3460) || defined(CONFIG_DRM_PTN3460_MODULE)

int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,

•           struct i2c_client *client, struct device_node *node);
  

•           struct i2c_client *client, struct device_node *node,
  

•           struct drm_panel *panel);
  

#else

static inline int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder, struct i2c_client *client,

•           struct device_node *node)
  

•           struct device_node *node, struct drm_panel *panel)
  

{ return 0; } -- 1.7.9.5

---

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

Hi Jingoo,

On Tue, Apr 22, 2014 at 2:13 PM, Jingoo Han jg1.han@samsung.com wrote:

On Tuesday, April 22, 2014 7:39 AM, Ajay Kumar wrote:

...

This patch adds a drm_bridge driver for the PS8622 DisplayPort to LVDS bridge chip.

Signed-off-by: Andrew Bresticker abrestic@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Rahul Sharma rahul.sharma@samsung.com Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com

---

Changes since V1: Pushing V1 for this as V2 because this patch holds good in this

series.

...

drivers/gpu/drm/bridge/Kconfig | 7 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/ps8622.c | 566

+++++++++++++++++++++++++++++++++++++++

...

include/drm/bridge/ps8622.h | 42 +++ 4 files changed, 616 insertions(+) create mode 100644 drivers/gpu/drm/bridge/ps8622.c create mode 100644 include/drm/bridge/ps8622.h

diff --git a/drivers/gpu/drm/bridge/Kconfig

b/drivers/gpu/drm/bridge/Kconfig

...

index 3bc6845..aba21fc 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -4,3 +4,10 @@ config DRM_PTN3460 select DRM_KMS_HELPER select DRM_PANEL ---help---

+config DRM_PS8622

• tristate "Parade eDP/LVDS bridge"
  

• depends on DRM
  

• select DRM_KMS_HELPER
  

• select DRM_PANEL
  

Please add the following select.

•   select BACKLIGHT_LCD_SUPPORT
  

•   select BACKLIGHT_CLASS_DEVICE
  
  

Without this, the following build issues happen.

drivers/gpu/drm/bridge/ps8622.c:353: undefined reference to `backlight_device_unregister' drivers/built-in.o: In function `ps8622_init': drivers/gpu/drm/bridge/ps8622.c:559: undefined reference to `backlight_device_unregister' drivers/gpu/drm/bridge/ps8622.c:507: undefined reference to `backlight_device_register'

Right. I will add it.

...

• ---help---
  

diff --git a/drivers/gpu/drm/bridge/Makefile

b/drivers/gpu/drm/bridge/Makefile

...

index b4733e1..d1b5daa 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -1,3 +1,4 @@ ccflags-y := -Iinclude/drm

obj-$(CONFIG_DRM_PTN3460) += ptn3460.o +obj-$(CONFIG_DRM_PS8622) += ps8622.o diff --git a/drivers/gpu/drm/bridge/ps8622.c

b/drivers/gpu/drm/bridge/ps8622.c

...

new file mode 100644 index 0000000..1e6b3ca --- /dev/null +++ b/drivers/gpu/drm/bridge/ps8622.c

[.....]

...

+static int ps8622_send_config(struct ps8622_bridge *ps_bridge) +{

• struct i2c_client *cl = ps_bridge->client;
  

• int err = 0;
  

• /* wait 20ms after power ON */
  

• usleep_range(20000, 30000);
  

• err |= ps8622_set(cl, 0x02, 0xa1, 0x01); /* HPD low */
  

• /* SW setting */
  

• err |= ps8622_set(cl, 0x04, 0x14, 0x01); /* [1:0] SW output 1.2V
  

voltage

...

•                                           * is lower to 96% */
  

The comment style looks awkward. Please choose one of two options. And change all comments in this file.

•   /* SW setting */
  

•   err |= ps8622_set(cl, 0x04, 0x14, 0x01); /* [1:0] SW output 1.2V
  

voltage is lower to 96% */

•   /* SW setting */
  

•   /* [1:0] SW output 1.2V voltage is lower to 96% */
  

•   err |= ps8622_set(cl, 0x04, 0x14, 0x01);
  
  

Will use this.

...

• /* RCO SS setting */
  

• err |= ps8622_set(cl, 0x04, 0xe3, 0x20); /* [5:4] = b01 0.5%, b10
  

1%,

...

•                                           * b11 1.5% */
  

• err |= ps8622_set(cl, 0x04, 0xe2, 0x80); /* [7] RCO SS enable */
  

• /* RPHY Setting */
  

• err |= ps8622_set(cl, 0x04, 0x8a, 0x0c); /* [3:2] CDR tune wait
  

cycle

...

•                                           * before measure for
  

fine tune

...

•                                           * b00: 1us b01: 0.5us
  

b10:2us

...

•                                           * b11: 4us */
  

• err |= ps8622_set(cl, 0x04, 0x89, 0x08); /* [3] RFD always on */
  

• err |= ps8622_set(cl, 0x04, 0x71, 0x2d); /* CTN lock in/out:
  

•                                           * 20000ppm/80000ppm.
  

•                                           * Lock out 2 times. */
  

• /* 2.7G CDR settings */
  

• err |= ps8622_set(cl, 0x04, 0x7d, 0x07); /* NOF=40LSB for HBR CDR
  

•                                           * setting */
  

• err |= ps8622_set(cl, 0x04, 0x7b, 0x00); /* [1:0] Fmin=+4bands */
  

• err |= ps8622_set(cl, 0x04, 0x7a, 0xfd); /* [7:5] DCO_FTRNG=+-40%
  

*/

...

• /* 1.62G CDR settings */
  

• err |= ps8622_set(cl, 0x04, 0xc0, 0x12); /* [5:2]NOF=64LSB [1:0]DCO
  

•                                           * scale is 2/5 */
  

• err |= ps8622_set(cl, 0x04, 0xc1, 0x92); /* Gitune=-37% */
  

• err |= ps8622_set(cl, 0x04, 0xc2, 0x1c); /* Fbstep=100% */
  

• err |= ps8622_set(cl, 0x04, 0x32, 0x80); /* [7] LOS signal disable
  

*/

...

• /* RPIO Setting */
  

• err |= ps8622_set(cl, 0x04, 0x00, 0xb0); /* [7:4] LVDS driver bias
  

•                                           * current : 75% (250mV
  

swing)

...

•                                           * */
  

• err |= ps8622_set(cl, 0x04, 0x15, 0x40); /* [7:6] Right-bar GPIO
  

output

...

•                                           * strength is 8mA */
  

• /* EQ Training State Machine Setting */
  

• err |= ps8622_set(cl, 0x04, 0x54, 0x10); /* RCO calibration start
  

*/

...

• /* Logic, needs more than 10 I2C command */
  

• err |= ps8622_set(cl, 0x01, 0x02, 0x80 |
  

ps_bridge->max_lane_count);

...

•                                          /* [4:0] MAX_LANE_COUNT
  

set to

...

•                                           * max supported lanes */
  

• err |= ps8622_set(cl, 0x01, 0x21, 0x80 | ps_bridge->lane_count);
  

•                                          /* [4:0] LANE_COUNT_SET
  

set to

...

•                                           * chosen lane count */
  

• err |= ps8622_set(cl, 0x00, 0x52, 0x20);
  

• err |= ps8622_set(cl, 0x00, 0xf1, 0x03); /* HPD CP toggle enable */
  

• err |= ps8622_set(cl, 0x00, 0x62, 0x41);
  

• err |= ps8622_set(cl, 0x00, 0xf6, 0x01); /* Counter number, add 1ms
  

•                                           * counter delay */
  

• err |= ps8622_set(cl, 0x00, 0x77, 0x06); /* [6]PWM function
  

control by

...

•                                           * DPCD0040f[7], default
  

is PWM

...

•                                           * block always works. */
  

• err |= ps8622_set(cl, 0x00, 0x4c, 0x04); /* 04h Adjust VTotal
  

tolerance

...

•                                           * to fix the 30Hz no
  

display

...

•                                           * issue */
  

• err |= ps8622_set(cl, 0x01, 0xc0, 0x00); /* DPCD00400='h00, Parade
  

OUI =

...

•                                           * 'h001cf8 */
  

• err |= ps8622_set(cl, 0x01, 0xc1, 0x1c); /* DPCD00401='h1c */
  

• err |= ps8622_set(cl, 0x01, 0xc2, 0xf8); /* DPCD00402='hf8 */
  

• err |= ps8622_set(cl, 0x01, 0xc3, 0x44); /* DPCD403~408 = ASCII
  

code

...

•                                           * D2SLV5='h4432534c5635
  

*/

...

• err |= ps8622_set(cl, 0x01, 0xc4, 0x32); /* DPCD404 */
  

• err |= ps8622_set(cl, 0x01, 0xc5, 0x53); /* DPCD405 */
  

• err |= ps8622_set(cl, 0x01, 0xc6, 0x4c); /* DPCD406 */
  

• err |= ps8622_set(cl, 0x01, 0xc7, 0x56); /* DPCD407 */
  

• err |= ps8622_set(cl, 0x01, 0xc8, 0x35); /* DPCD408 */
  

• err |= ps8622_set(cl, 0x01, 0xca, 0x01); /* DPCD40A, Initial Code
  

major

...

•                                           * revision '01' */
  

• err |= ps8622_set(cl, 0x01, 0xcb, 0x05); /* DPCD40B, Initial Code
  

minor

...

•                                           * revision '05' */
  

• if (ps_bridge->bl) {
  

•         err |= ps8622_set(cl, 0x01, 0xa5, 0xa0);
  

•                                         /* DPCD720, internal PWM */
  

•         err |= ps8622_set(cl, 0x01, 0xa7,
  

•                         ps_bridge->bl->props.brightness);
  

•                                          /* FFh for 100%
  

brightness,

...

•                                           *  0h for 0% brightness
  

*/

...

• } else {
  

•         err |= ps8622_set(cl, 0x01, 0xa5, 0x80);
  

•                                         /* DPCD720, external PWM */
  

• }
  

• err |= ps8622_set(cl, 0x01, 0xcc, 0x13); /* Set LVDS output as
  

6bit-VESA

...

•                                           * mapping, single LVDS
  

channel

...

•                                           * */
  

• err |= ps8622_set(cl, 0x02, 0xb1, 0x20); /* Enable SSC set by
  

register

...

•                                           * */
  

• err |= ps8622_set(cl, 0x04, 0x10, 0x16); /* Set SSC enabled and
  

+/-1%

...

•                                           * central spreading */
  

• /* Logic end */
  

• err |= ps8622_set(cl, 0x04, 0x59, 0x60); /* MPU Clock source: LC
  

=> RCO

...

•                                           * */
  

• err |= ps8622_set(cl, 0x04, 0x54, 0x14); /* LC -> RCO */
  

• err |= ps8622_set(cl, 0x02, 0xa1, 0x91); /* HPD high */
  

• return err ? -EIO : 0;
  

+}

[.....]

...

+static void ps8622_pre_enable(struct drm_bridge *bridge) +{

• struct ps8622_bridge *ps_bridge = bridge->driver_private;
  

• int ret;
  

• mutex_lock(&ps_bridge->enable_mutex);
  

• if (ps_bridge->enabled)
  

•         goto out;
  

• if (gpio_is_valid(ps_bridge->gpio_rst_n))
  

•         gpio_set_value(ps_bridge->gpio_rst_n, 0);
  

• if (ps_bridge->v12) {
  

•         ret = regulator_enable(ps_bridge->v12);
  

•         if (ret)
  

•                 DRM_ERROR("fails to enable ps_bridge->v12");
  

• }
  

• drm_panel_pre_enable(ps_bridge->panel);
  

• if (gpio_is_valid(ps_bridge->gpio_slp_n))
  

•         gpio_set_value(ps_bridge->gpio_slp_n, 1);
  

• /*
  

•  * T1 is the range of time that it takes for the power to rise
  

after we

...

•  * enable the lcd fet. T2 is the range of time in which the data
  

sheet

...

•  * specifies we should deassert the reset pin.
  

•  *
  

•  * If it takes T1.max for the power to rise, we need to wait
  

atleast

...

•  * T2.min before deasserting the reset pin. If it takes T1.min for
  

the

...

•  * power to rise, we need to wait at most T2.max before
  

deasserting the

...

•  * reset pin.
  

•  */
  

• usleep_range(PS8622_RST_HIGH_T2_MIN_US +
  

PS8622_POWER_RISE_T1_MAX_US,

...

•              PS8622_RST_HIGH_T2_MAX_US +
  

PS8622_POWER_RISE_T1_MIN_US);

...

• if (gpio_is_valid(ps_bridge->gpio_rst_n))
  

•         gpio_set_value(ps_bridge->gpio_rst_n, 1);
  

• ret = ps8622_send_config(ps_bridge);
  

• if (ret)
  

•         DRM_ERROR("Failed to send config to bridge (%d)\n", ret);
  

• ps_bridge->enabled = true;
  

• mutex_unlock(&ps_bridge->enable_mutex);
  

• return;
  

+out:

• mutex_unlock(&ps_bridge->enable_mutex);
  

+}

mutex_unlock() is duplicated. Also, 'return' is unnecessary. Please fix it as below.

•   ps_bridge->enabled = true;
  

+out:

•   mutex_unlock(&ps_bridge->enable_mutex);
  

+}

Right. Will change it.

...

+static void ps8622_enable(struct drm_bridge *bridge) +{

• struct ps8622_bridge *ps_bridge = bridge->driver_private;
  

• mutex_lock(&ps_bridge->enable_mutex);
  

• if (ps_bridge->enabled)
  

•         drm_panel_enable(ps_bridge->panel);
  

• mutex_unlock(&ps_bridge->enable_mutex);
  

+}

+static void ps8622_disable(struct drm_bridge *bridge) +{

• struct ps8622_bridge *ps_bridge = bridge->driver_private;
  

• mutex_lock(&ps_bridge->enable_mutex);
  

• if (!ps_bridge->enabled)
  

•         goto out;
  

• ps_bridge->enabled = false;
  

• drm_panel_disable(ps_bridge->panel);
  

• msleep(PS8622_PWMO_END_T12_MS);
  

• /*
  

•  * This doesn't matter if the regulators are turned off, but
  

something

...

•  * else might keep them on. In that case, we want to assert the
  

slp gpio

...

•  * to lower power.
  

•  */
  

• if (gpio_is_valid(ps_bridge->gpio_slp_n))
  

•         gpio_set_value(ps_bridge->gpio_slp_n, 0);
  

• drm_panel_post_disable(ps_bridge->panel);
  

• if (ps_bridge->v12)
  

•         regulator_disable(ps_bridge->v12);
  

• /*
  

•  * Sleep for at least the amount of time that it takes the power
  

rail to

...

•  * fall to prevent asserting the rst gpio from doing anything.
  

•  */
  

• usleep_range(PS8622_POWER_FALL_T16_MAX_US,
  

•              2 * PS8622_POWER_FALL_T16_MAX_US);
  

• if (gpio_is_valid(ps_bridge->gpio_rst_n))
  

•         gpio_set_value(ps_bridge->gpio_rst_n, 0);
  

• msleep(PS8622_POWER_OFF_T17_MS);
  

+out:

• mutex_unlock(&ps_bridge->enable_mutex);
  

+}

+static void ps8622_post_disable(struct drm_bridge *bridge) +{ +}

How about just removing this empty function?

That is not possible. Because the bridge callbacks are called in this fashion: bridge->funcs->post_disable(bridge); So, if that particular callback turns NULL, it will crash!

[.....]

Best regards, Jingoo Han

Regards, Ajay Kumar

Hi Sean,

On Tue, Apr 22, 2014 at 7:01 PM, Sean Paul seanpaul@google.com wrote:

On Mon, Apr 21, 2014 at 6:39 PM, Ajay Kumar ajaykumar.rs@samsung.com wrote:

...

This patch adds a drm_bridge driver for the PS8622 DisplayPort to LVDS bridge chip.

Signed-off-by: Andrew Bresticker abrestic@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Rahul Sharma rahul.sharma@samsung.com Signed-off-by: Ajay Kumar ajaykumar.rs@samsung.com

Hi Ajay, I don't think that you should be claiming authorship of this driver, I don't see any commits from you in the chromium tree ([1] & [2]). The driver was originally written by vpalatin@chromium.org (who's completely missing from SoB), it should probably be attributed to him.

Sean

Oops. I am really sorry and I didn't really want to claim the authorship for this patch. I missed it while doing "git format-patch". And, thanks for letting me know that Vincent was the actual author. I will add his authorship.

Regards, Ajay Kumar

[1]- https://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel-next.git;a=... [2]- https://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel-next.git;a=...

...

---

Changes since V1: Pushing V1 for this as V2 because this patch holds good in this series.

drivers/gpu/drm/bridge/Kconfig | 7 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/ps8622.c | 566 +++++++++++++++++++++++++++++++++++++++ include/drm/bridge/ps8622.h | 42 +++ 4 files changed, 616 insertions(+) create mode 100644 drivers/gpu/drm/bridge/ps8622.c create mode 100644 include/drm/bridge/ps8622.h

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 3bc6845..aba21fc 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -4,3 +4,10 @@ config DRM_PTN3460 select DRM_KMS_HELPER select DRM_PANEL ---help---

+config DRM_PS8622

•   tristate "Parade eDP/LVDS bridge"
  

•   depends on DRM
  

•   select DRM_KMS_HELPER
  

•   select DRM_PANEL
  

•   ---help---
  

diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index b4733e1..d1b5daa 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -1,3 +1,4 @@ ccflags-y := -Iinclude/drm

obj-$(CONFIG_DRM_PTN3460) += ptn3460.o +obj-$(CONFIG_DRM_PS8622) += ps8622.o diff --git a/drivers/gpu/drm/bridge/ps8622.c b/drivers/gpu/drm/bridge/ps8622.c new file mode 100644 index 0000000..1e6b3ca --- /dev/null +++ b/drivers/gpu/drm/bridge/ps8622.c @@ -0,0 +1,566 @@ +/*

• • Parade PS8622 eDP/LVDS bridge driver
• • Copyright (C) 2014 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 <linux/module.h> +#include <linux/backlight.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/fb.h> +#include <linux/gpio.h> +#include <linux/i2c.h> +#include <linux/of.h> +#include <linux/of_gpio.h> +#include <linux/pm.h> +#include <linux/regulator/consumer.h> +#include <drm/drm_panel.h>

+#include "drmP.h" +#include "drm_crtc.h" +#include "drm_crtc_helper.h"

+struct ps8622_bridge {

•   struct drm_connector connector;
  

•   struct drm_bridge *bridge;
  

•   struct drm_encoder *encoder;
  

•   struct drm_panel *panel;
  

•   struct i2c_client *client;
  

•   struct regulator *v12;
  

•   struct backlight_device *bl;
  

•   struct mutex enable_mutex;
  

•   int gpio_slp_n;
  

•   int gpio_rst_n;
  

•   u8 max_lane_count;
  

•   u8 lane_count;
  

•   bool enabled;
  

•   struct drm_display_mode mode;
  

+};

+struct ps8622_device_data {

•   u8 max_lane_count;
  

+};

+static const struct ps8622_device_data ps8622_data = {

•   .max_lane_count = 1,
  

+};

+static const struct ps8622_device_data ps8625_data = {

•   .max_lane_count = 2,
  

+};

+/* Brightness scale on the Parade chip */ +#define PS8622_MAX_BRIGHTNESS 0xff

+/* Timings taken from the version 1.7 datasheet for the PS8622/PS8625 */ +#define PS8622_POWER_RISE_T1_MIN_US 10 +#define PS8622_POWER_RISE_T1_MAX_US 10000 +#define PS8622_RST_HIGH_T2_MIN_US 3000 +#define PS8622_RST_HIGH_T2_MAX_US 30000 +#define PS8622_PWMO_END_T12_MS 200 +#define PS8622_POWER_FALL_T16_MAX_US 10000 +#define PS8622_POWER_OFF_T17_MS 500

+#if ((PS8622_RST_HIGH_T2_MIN_US + PS8622_POWER_RISE_T1_MAX_US) > \

•   (PS8622_RST_HIGH_T2_MAX_US + PS8622_POWER_RISE_T1_MIN_US))
  

+#error "T2.min + T1.max must be less than T2.max + T1.min" +#endif

+static int ps8622_set(struct i2c_client *client, u8 page, u8 reg, u8 val) +{

•   int ret;
  

•   struct i2c_adapter *adap = client->adapter;
  

•   struct i2c_msg msg;
  

•   u8 data[] = {reg, val};
  

•   msg.addr = client->addr + page;
  

•   msg.flags = 0;
  

•   msg.len = sizeof(data);
  

•   msg.buf = data;
  

•   ret = i2c_transfer(adap, &msg, 1);
  

•   if (ret != 1)
  

•           pr_warn("PS8622 I2C write (0x%02x,0x%02x,0x%02x) failed: %d\n",
  

•                   client->addr + page, reg, val, ret);
  

•   return !(ret == 1);
  

+}

+static int ps8622_send_config(struct ps8622_bridge *ps_bridge) +{

•   struct i2c_client *cl = ps_bridge->client;
  

•   int err = 0;
  

•   /* wait 20ms after power ON */
  

•   usleep_range(20000, 30000);
  

•   err |= ps8622_set(cl, 0x02, 0xa1, 0x01); /* HPD low */
  

•   /* SW setting */
  

•   err |= ps8622_set(cl, 0x04, 0x14, 0x01); /* [1:0] SW output 1.2V voltage
  

•                                             * is lower to 96% */
  

•   /* RCO SS setting */
  

•   err |= ps8622_set(cl, 0x04, 0xe3, 0x20); /* [5:4] = b01 0.5%, b10 1%,
  

•                                             * b11 1.5% */
  

•   err |= ps8622_set(cl, 0x04, 0xe2, 0x80); /* [7] RCO SS enable */
  

•   /* RPHY Setting */
  

•   err |= ps8622_set(cl, 0x04, 0x8a, 0x0c); /* [3:2] CDR tune wait cycle
  

•                                             * before measure for fine tune
  

•                                             * b00: 1us b01: 0.5us b10:2us
  

•                                             * b11: 4us */
  

•   err |= ps8622_set(cl, 0x04, 0x89, 0x08); /* [3] RFD always on */
  

•   err |= ps8622_set(cl, 0x04, 0x71, 0x2d); /* CTN lock in/out:
  

•                                             * 20000ppm/80000ppm.
  

•                                             * Lock out 2 times. */
  

•   /* 2.7G CDR settings */
  

•   err |= ps8622_set(cl, 0x04, 0x7d, 0x07); /* NOF=40LSB for HBR CDR
  

•                                             * setting */
  

•   err |= ps8622_set(cl, 0x04, 0x7b, 0x00); /* [1:0] Fmin=+4bands */
  

•   err |= ps8622_set(cl, 0x04, 0x7a, 0xfd); /* [7:5] DCO_FTRNG=+-40% */
  

•   /* 1.62G CDR settings */
  

•   err |= ps8622_set(cl, 0x04, 0xc0, 0x12); /* [5:2]NOF=64LSB [1:0]DCO
  

•                                             * scale is 2/5 */
  

•   err |= ps8622_set(cl, 0x04, 0xc1, 0x92); /* Gitune=-37% */
  

•   err |= ps8622_set(cl, 0x04, 0xc2, 0x1c); /* Fbstep=100% */
  

•   err |= ps8622_set(cl, 0x04, 0x32, 0x80); /* [7] LOS signal disable */
  

•   /* RPIO Setting */
  

•   err |= ps8622_set(cl, 0x04, 0x00, 0xb0); /* [7:4] LVDS driver bias
  

•                                             * current : 75% (250mV swing)
  

•                                             * */
  

•   err |= ps8622_set(cl, 0x04, 0x15, 0x40); /* [7:6] Right-bar GPIO output
  

•                                             * strength is 8mA */
  

•   /* EQ Training State Machine Setting */
  

•   err |= ps8622_set(cl, 0x04, 0x54, 0x10); /* RCO calibration start */
  

•   /* Logic, needs more than 10 I2C command */
  

•   err |= ps8622_set(cl, 0x01, 0x02, 0x80 | ps_bridge->max_lane_count);
  

•                                            /* [4:0] MAX_LANE_COUNT set to
  

•                                             * max supported lanes */
  

•   err |= ps8622_set(cl, 0x01, 0x21, 0x80 | ps_bridge->lane_count);
  

•                                            /* [4:0] LANE_COUNT_SET set to
  

•                                             * chosen lane count */
  

•   err |= ps8622_set(cl, 0x00, 0x52, 0x20);
  

•   err |= ps8622_set(cl, 0x00, 0xf1, 0x03); /* HPD CP toggle enable */
  

•   err |= ps8622_set(cl, 0x00, 0x62, 0x41);
  

•   err |= ps8622_set(cl, 0x00, 0xf6, 0x01); /* Counter number, add 1ms
  

•                                             * counter delay */
  

•   err |= ps8622_set(cl, 0x00, 0x77, 0x06); /* [6]PWM function control by
  

•                                             * DPCD0040f[7], default is PWM
  

•                                             * block always works. */
  

•   err |= ps8622_set(cl, 0x00, 0x4c, 0x04); /* 04h Adjust VTotal tolerance
  

•                                             * to fix the 30Hz no display
  

•                                             * issue */
  

•   err |= ps8622_set(cl, 0x01, 0xc0, 0x00); /* DPCD00400='h00, Parade OUI =
  

•                                             * 'h001cf8 */
  

•   err |= ps8622_set(cl, 0x01, 0xc1, 0x1c); /* DPCD00401='h1c */
  

•   err |= ps8622_set(cl, 0x01, 0xc2, 0xf8); /* DPCD00402='hf8 */
  

•   err |= ps8622_set(cl, 0x01, 0xc3, 0x44); /* DPCD403~408 = ASCII code
  

•                                             * D2SLV5='h4432534c5635 */
  

•   err |= ps8622_set(cl, 0x01, 0xc4, 0x32); /* DPCD404 */
  

•   err |= ps8622_set(cl, 0x01, 0xc5, 0x53); /* DPCD405 */
  

•   err |= ps8622_set(cl, 0x01, 0xc6, 0x4c); /* DPCD406 */
  

•   err |= ps8622_set(cl, 0x01, 0xc7, 0x56); /* DPCD407 */
  

•   err |= ps8622_set(cl, 0x01, 0xc8, 0x35); /* DPCD408 */
  

•   err |= ps8622_set(cl, 0x01, 0xca, 0x01); /* DPCD40A, Initial Code major
  

•                                             * revision '01' */
  

•   err |= ps8622_set(cl, 0x01, 0xcb, 0x05); /* DPCD40B, Initial Code minor
  

•                                             * revision '05' */
  

•   if (ps_bridge->bl) {
  

•           err |= ps8622_set(cl, 0x01, 0xa5, 0xa0);
  

•                                           /* DPCD720, internal PWM */
  

•           err |= ps8622_set(cl, 0x01, 0xa7,
  

•                           ps_bridge->bl->props.brightness);
  

•                                            /* FFh for 100% brightness,
  

•                                             *  0h for 0% brightness */
  

•   } else {
  

•           err |= ps8622_set(cl, 0x01, 0xa5, 0x80);
  

•                                           /* DPCD720, external PWM */
  

•   }
  

•   err |= ps8622_set(cl, 0x01, 0xcc, 0x13); /* Set LVDS output as 6bit-VESA
  

•                                             * mapping, single LVDS channel
  

•                                             * */
  

•   err |= ps8622_set(cl, 0x02, 0xb1, 0x20); /* Enable SSC set by register
  

•                                             * */
  

•   err |= ps8622_set(cl, 0x04, 0x10, 0x16); /* Set SSC enabled and +/-1%
  

•                                             * central spreading */
  

•   /* Logic end */
  

•   err |= ps8622_set(cl, 0x04, 0x59, 0x60); /* MPU Clock source: LC => RCO
  

•                                             * */
  

•   err |= ps8622_set(cl, 0x04, 0x54, 0x14); /* LC -> RCO */
  

•   err |= ps8622_set(cl, 0x02, 0xa1, 0x91); /* HPD high */
  

•   return err ? -EIO : 0;
  

+}

+static int ps8622_backlight_update(struct backlight_device *bl) +{

•   struct ps8622_bridge *ps_bridge = dev_get_drvdata(&bl->dev);
  

•   int ret, brightness = bl->props.brightness;
  

•   if (bl->props.power != FB_BLANK_UNBLANK ||
  

•       bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
  

•           brightness = 0;
  

•   mutex_lock(&ps_bridge->enable_mutex);
  

•   if (!ps_bridge->enabled) {
  

•           ret = -EINVAL;
  

•           goto out;
  

•   }
  

•   ret = ps8622_set(ps_bridge->client, 0x01, 0xa7, brightness);
  

+out:

•   mutex_unlock(&ps_bridge->enable_mutex);
  

•   return ret;
  

+}

+static int ps8622_backlight_get(struct backlight_device *bl) +{

•   return bl->props.brightness;
  

+}

+static const struct backlight_ops ps8622_backlight_ops = {

•   .update_status  = ps8622_backlight_update,
  

•   .get_brightness = ps8622_backlight_get,
  

+};

+static void ps8622_pre_enable(struct drm_bridge *bridge) +{

•   struct ps8622_bridge *ps_bridge = bridge->driver_private;
  

•   int ret;
  

•   mutex_lock(&ps_bridge->enable_mutex);
  

•   if (ps_bridge->enabled)
  

•           goto out;
  

•   if (gpio_is_valid(ps_bridge->gpio_rst_n))
  

•           gpio_set_value(ps_bridge->gpio_rst_n, 0);
  

•   if (ps_bridge->v12) {
  

•           ret = regulator_enable(ps_bridge->v12);
  

•           if (ret)
  

•                   DRM_ERROR("fails to enable ps_bridge->v12");
  

•   }
  

•   drm_panel_pre_enable(ps_bridge->panel);
  

•   if (gpio_is_valid(ps_bridge->gpio_slp_n))
  

•           gpio_set_value(ps_bridge->gpio_slp_n, 1);
  

•   /*
  

•    * T1 is the range of time that it takes for the power to rise after we
  

•    * enable the lcd fet. T2 is the range of time in which the data sheet
  

•    * specifies we should deassert the reset pin.
  

•    *
  

•    * If it takes T1.max for the power to rise, we need to wait atleast
  

•    * T2.min before deasserting the reset pin. If it takes T1.min for the
  

•    * power to rise, we need to wait at most T2.max before deasserting the
  

•    * reset pin.
  

•    */
  

•   usleep_range(PS8622_RST_HIGH_T2_MIN_US + PS8622_POWER_RISE_T1_MAX_US,
  

•                PS8622_RST_HIGH_T2_MAX_US + PS8622_POWER_RISE_T1_MIN_US);
  

•   if (gpio_is_valid(ps_bridge->gpio_rst_n))
  

•           gpio_set_value(ps_bridge->gpio_rst_n, 1);
  

•   ret = ps8622_send_config(ps_bridge);
  

•   if (ret)
  

•           DRM_ERROR("Failed to send config to bridge (%d)\n", ret);
  

•   ps_bridge->enabled = true;
  

•   mutex_unlock(&ps_bridge->enable_mutex);
  

•   return;
  

+out:

•   mutex_unlock(&ps_bridge->enable_mutex);
  

+}

+static void ps8622_enable(struct drm_bridge *bridge) +{

•   struct ps8622_bridge *ps_bridge = bridge->driver_private;
  

•   mutex_lock(&ps_bridge->enable_mutex);
  

•   if (ps_bridge->enabled)
  

•           drm_panel_enable(ps_bridge->panel);
  

•   mutex_unlock(&ps_bridge->enable_mutex);
  

+}

+static void ps8622_disable(struct drm_bridge *bridge) +{

•   struct ps8622_bridge *ps_bridge = bridge->driver_private;
  

•   mutex_lock(&ps_bridge->enable_mutex);
  

•   if (!ps_bridge->enabled)
  

•           goto out;
  

•   ps_bridge->enabled = false;
  

•   drm_panel_disable(ps_bridge->panel);
  

•   msleep(PS8622_PWMO_END_T12_MS);
  

•   /*
  

•    * This doesn't matter if the regulators are turned off, but something
  

•    * else might keep them on. In that case, we want to assert the slp gpio
  

•    * to lower power.
  

•    */
  

•   if (gpio_is_valid(ps_bridge->gpio_slp_n))
  

•           gpio_set_value(ps_bridge->gpio_slp_n, 0);
  

•   drm_panel_post_disable(ps_bridge->panel);
  

•   if (ps_bridge->v12)
  

•           regulator_disable(ps_bridge->v12);
  

•   /*
  

•    * Sleep for at least the amount of time that it takes the power rail to
  

•    * fall to prevent asserting the rst gpio from doing anything.
  

•    */
  

•   usleep_range(PS8622_POWER_FALL_T16_MAX_US,
  

•                2 * PS8622_POWER_FALL_T16_MAX_US);
  

•   if (gpio_is_valid(ps_bridge->gpio_rst_n))
  

•           gpio_set_value(ps_bridge->gpio_rst_n, 0);
  

•   msleep(PS8622_POWER_OFF_T17_MS);
  

+out:

•   mutex_unlock(&ps_bridge->enable_mutex);
  

+}

+static void ps8622_post_disable(struct drm_bridge *bridge) +{ +}

+static void ps8622_destroy(struct drm_bridge *bridge) +{

•   struct ps8622_bridge *ps_bridge = bridge->driver_private;
  

•   drm_bridge_cleanup(bridge);
  

•   if (ps_bridge->bl)
  

•           backlight_device_unregister(ps_bridge->bl);
  

•   put_device(&ps_bridge->client->dev);
  

+}

+static const struct drm_bridge_funcs ps8622_bridge_funcs = {

•   .pre_enable = ps8622_pre_enable,
  

•   .enable = ps8622_enable,
  

•   .disable = ps8622_disable,
  

•   .post_disable = ps8622_post_disable,
  

•   .destroy = ps8622_destroy,
  

+};

+static int ps8622_get_modes(struct drm_connector *connector) +{

•   struct ps8622_bridge *ps_bridge;
  

•   struct drm_display_mode *new_mode;
  

•   ps_bridge = container_of(connector, struct ps8622_bridge, connector);
  

•   new_mode = drm_mode_duplicate(connector->dev, &ps_bridge->mode);
  

•   if (!new_mode) {
  

•           DRM_ERROR("Failed to duplicate ps_bridge mode!\n");
  

•           return 0;
  

•   }
  

•   drm_mode_probed_add(connector, new_mode);
  

•   return 1;
  

+}

+static int ps8622_mode_valid(struct drm_connector *connector,

•           struct drm_display_mode *mode)
  

+{

•   struct ps8622_bridge *ps_bridge;
  

•   ps_bridge = container_of(connector, struct ps8622_bridge, connector);
  

•   return drm_mode_equal(mode, &ps_bridge->mode) ? MODE_OK : MODE_BAD;
  

+}

+static struct drm_encoder *ps8622_best_encoder(struct drm_connector *connector) +{

•   struct ps8622_bridge *ps_bridge;
  

•   ps_bridge = container_of(connector, struct ps8622_bridge, connector);
  

•   return ps_bridge->encoder;
  

+}

+static const struct drm_connector_helper_funcs ps8622_connector_helper_funcs = {

•   .get_modes = ps8622_get_modes,
  

•   .mode_valid = ps8622_mode_valid,
  

•   .best_encoder = ps8622_best_encoder,
  

+};

+static enum drm_connector_status ps8622_detect(struct drm_connector *connector,

•           bool force)
  

+{

•   return connector_status_connected;
  

+}

+static void ps8622_connector_destroy(struct drm_connector *connector) +{

•   drm_connector_cleanup(connector);
  

+}

+static const struct drm_connector_funcs ps8622_connector_funcs = {

•   .dpms = drm_helper_connector_dpms,
  

•   .fill_modes = drm_helper_probe_single_connector_modes,
  

•   .detect = ps8622_detect,
  

•   .destroy = ps8622_connector_destroy,
  

+};

+static const struct of_device_id ps8622_devices[] = {

•   {
  

•           .compatible = "parade,ps8622",
  

•           .data   = &ps8622_data,
  

•   }, {
  

•           .compatible = "parade,ps8625",
  

•           .data   = &ps8625_data,
  

•   }, {
  

•           /* end node */
  

•   }
  

+};

+int ps8622_init(struct drm_device *dev, struct drm_encoder *encoder,

•           struct i2c_client *client, struct device_node *node,
  

•           struct drm_panel *panel)
  

+{

•   int ret;
  

•   const struct of_device_id *match;
  

•   const struct ps8622_device_data *device_data;
  

•   struct drm_bridge *bridge;
  

•   struct ps8622_bridge *ps_bridge;
  

•   node = of_find_matching_node_and_match(NULL, ps8622_devices, &match);
  

•   if (!node)
  

•           return -ENODEV;
  

•   bridge = devm_kzalloc(dev->dev, sizeof(*bridge), GFP_KERNEL);
  

•   if (!bridge) {
  

•           DRM_ERROR("Failed to allocate drm bridge\n");
  

•           return -ENOMEM;
  

•   }
  

•   ps_bridge = devm_kzalloc(dev->dev, sizeof(*ps_bridge), GFP_KERNEL);
  

•   if (!ps_bridge) {
  

•           DRM_ERROR("could not allocate ps bridge\n");
  

•           return -ENOMEM;
  

•   }
  

•   mutex_init(&ps_bridge->enable_mutex);
  

•   device_data = match->data;
  

•   ps_bridge->client = client;
  

•   ps_bridge->encoder = encoder;
  

•   ps_bridge->bridge = bridge;
  

•   ps_bridge->v12 = devm_regulator_get(&client->dev, "vdd_bridge");
  

•   if (IS_ERR(ps_bridge->v12)) {
  

•           DRM_INFO("no 1.2v regulator found for PS8622\n");
  

•           ps_bridge->v12 = NULL;
  

•   }
  

•   ps_bridge->gpio_slp_n = of_get_named_gpio(node, "sleep-gpio", 0);
  

•   if (gpio_is_valid(ps_bridge->gpio_slp_n)) {
  

•           ret = devm_gpio_request_one(&client->dev, ps_bridge->gpio_slp_n,
  

•                                       GPIOF_OUT_INIT_HIGH,
  

•                                       "PS8622_SLP_N");
  

•           if (ret)
  

•                   goto err_client;
  

•   }
  

•   ps_bridge->gpio_rst_n = of_get_named_gpio(node, "reset-gpio", 0);
  

•   if (gpio_is_valid(ps_bridge->gpio_rst_n)) {
  

•           /*
  

•            * Assert the reset pin high to avoid the bridge being
  

•            * initialized prematurely
  

•            */
  

•           ret = devm_gpio_request_one(&client->dev, ps_bridge->gpio_rst_n,
  

•                                       GPIOF_OUT_INIT_HIGH,
  

•                                       "PS8622_RST_N");
  

•           if (ret)
  

•                   goto err_client;
  

•   }
  

•   ps_bridge->max_lane_count = device_data->max_lane_count;
  

•   if (of_property_read_u8(node, "lane-count", &ps_bridge->lane_count))
  

•           ps_bridge->lane_count = ps_bridge->max_lane_count;
  

•   else if (ps_bridge->lane_count > ps_bridge->max_lane_count) {
  

•           DRM_ERROR("lane-count property is too high for DP bridge\n");
  

•           ps_bridge->lane_count = ps_bridge->max_lane_count;
  

•   }
  

•   if (!of_find_property(node, "use-external-pwm", NULL)) {
  

•           ps_bridge->bl = backlight_device_register("ps8622-backlight",
  

•                           dev->dev, ps_bridge, &ps8622_backlight_ops,
  

•                           NULL);
  

•           if (IS_ERR(ps_bridge->bl)) {
  

•                   DRM_ERROR("failed to register backlight\n");
  

•                   ret = PTR_ERR(ps_bridge->bl);
  

•                   ps_bridge->bl = NULL;
  

•                   goto err_client;
  

•           }
  

•           ps_bridge->bl->props.max_brightness = PS8622_MAX_BRIGHTNESS;
  

•           ps_bridge->bl->props.brightness = PS8622_MAX_BRIGHTNESS;
  

•   }
  

•   ret = of_get_drm_display_mode(node, &ps_bridge->mode, 0);
  

•   if (ret) {
  

•           DRM_ERROR("Failed to get display mode, ret=%d\n", ret);
  

•           goto err_backlight;
  

•   }
  

•   ret = drm_bridge_init(dev, ps_bridge->bridge, &ps8622_bridge_funcs);
  

•   if (ret) {
  

•           DRM_ERROR("Failed to initialize bridge with drm\n");
  

•           goto err_backlight;
  

•   }
  

•   if (panel) {
  

•           ps_bridge->panel = panel;
  

•           drm_panel_attach(ps_bridge->panel, &ps_bridge->connector);
  

•   }
  

•   ps_bridge->bridge->driver_private = ps_bridge;
  

•   encoder->bridge = ps_bridge->bridge;
  

•   ps_bridge->enabled = false;
  

•   ps_bridge->connector.polled = DRM_CONNECTOR_POLL_HPD;
  

•   ret = drm_connector_init(dev, &ps_bridge->connector,
  

•                   &ps8622_connector_funcs, DRM_MODE_CONNECTOR_LVDS);
  

•   if (ret) {
  

•           DRM_ERROR("Failed to initialize connector with drm\n");
  

•           goto err_bridge;
  

•   }
  

•   drm_connector_helper_add(&ps_bridge->connector,
  

•                   &ps8622_connector_helper_funcs);
  

•   drm_sysfs_connector_add(&ps_bridge->connector);
  

•   drm_mode_connector_attach_encoder(&ps_bridge->connector, encoder);
  

•   return 0;
  

+err_bridge:

•   drm_bridge_cleanup(ps_bridge->bridge);
  

+err_backlight:

•   if (ps_bridge->bl)
  

•           backlight_device_unregister(ps_bridge->bl);
  

+err_client:

•   put_device(&client->dev);
  

•   DRM_ERROR("device probe failed : %d\n", ret);
  

•   return ret;
  

+} +EXPORT_SYMBOL(ps8622_init); diff --git a/include/drm/bridge/ps8622.h b/include/drm/bridge/ps8622.h new file mode 100644 index 0000000..4fc1959 --- /dev/null +++ b/include/drm/bridge/ps8622.h @@ -0,0 +1,42 @@ +/*

• • Copyright (C) 2014 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_PS8622_H_ +#define _DRM_BRIDGE_PS8622_H_

+struct drm_device; +struct drm_encoder; +struct i2c_client; +struct device_node; +struct drm_panel;

+#ifdef CONFIG_DRM_PS8622

+extern int ps8622_init(struct drm_device *dev, struct drm_encoder *encoder,

•           struct i2c_client *client, struct device_node *node,
  

•           struct drm_panel *panel);
  

+#else

+static inline int ps8622_init(struct drm_device *dev,

•                           struct drm_encoder *encoder,
  

•                           struct i2c_client *client,
  

•                           struct device_node *node,
  

•                           struct drm_panel *panel)
  

+{

•   return -ENODEV;
  

+}

+#endif

+#endif

1.7.9.5