If we cannot find a panel, assume that the output from the PL111 is connected directly to a "dumb" VGA connector, so look up the connector from that bridge.
Signed-off-by: Linus Walleij linus.walleij@linaro.org --- This is how the new API is used in the PL111. --- drivers/gpu/drm/pl111/Kconfig | 1 + drivers/gpu/drm/pl111/pl111_drv.c | 3 +++ 2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig index e5e2abd66491..82cb3e60ddc8 100644 --- a/drivers/gpu/drm/pl111/Kconfig +++ b/drivers/gpu/drm/pl111/Kconfig @@ -8,6 +8,7 @@ config DRM_PL111 select DRM_GEM_CMA_HELPER select DRM_BRIDGE select DRM_PANEL_BRIDGE + select DRM_DUMB_VGA_DAC select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE help Choose this option for DRM support for the PL111 CLCD controller. diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c index f5bc6f160e60..6db423bbd84e 100644 --- a/drivers/gpu/drm/pl111/pl111_drv.c +++ b/drivers/gpu/drm/pl111/pl111_drv.c @@ -67,6 +67,7 @@ #include <drm/drm_of.h> #include <drm/drm_bridge.h> #include <drm/drm_panel.h> +#include <drm/dumb_vga_dac.h>
#include "pl111_drm.h" #include "pl111_versatile.h" @@ -128,6 +129,8 @@ static int pl111_modeset_init(struct drm_device *dev) if (panel) { priv->panel = panel; priv->connector = panel->connector; + } else { + priv->connector = drm_dumb_vga_get_connector(bridge); } priv->bridge = bridge;
On Fri, Sep 01, 2017 at 11:46:29AM +0200, Linus Walleij wrote:
If we cannot find a panel, assume that the output from the PL111 is connected directly to a "dumb" VGA connector, so look up the connector from that bridge.
Signed-off-by: Linus Walleij linus.walleij@linaro.org
This is how the new API is used in the PL111.
drivers/gpu/drm/pl111/Kconfig | 1 + drivers/gpu/drm/pl111/pl111_drv.c | 3 +++ 2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig index e5e2abd66491..82cb3e60ddc8 100644 --- a/drivers/gpu/drm/pl111/Kconfig +++ b/drivers/gpu/drm/pl111/Kconfig @@ -8,6 +8,7 @@ config DRM_PL111 select DRM_GEM_CMA_HELPER select DRM_BRIDGE select DRM_PANEL_BRIDGE
- select DRM_DUMB_VGA_DAC select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE help Choose this option for DRM support for the PL111 CLCD controller.
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c index f5bc6f160e60..6db423bbd84e 100644 --- a/drivers/gpu/drm/pl111/pl111_drv.c +++ b/drivers/gpu/drm/pl111/pl111_drv.c @@ -67,6 +67,7 @@ #include <drm/drm_of.h> #include <drm/drm_bridge.h> #include <drm/drm_panel.h> +#include <drm/dumb_vga_dac.h>
#include "pl111_drm.h" #include "pl111_versatile.h" @@ -128,6 +129,8 @@ static int pl111_modeset_init(struct drm_device *dev) if (panel) { priv->panel = panel; priv->connector = panel->connector;
- } else {
priv->connector = drm_dumb_vga_get_connector(bridge);
Why do you need to set this? The only code I could find tries to set polarity values from bus_flags, and for VGA I'd assume you want to instead key this off the mode?
Wrt the more general problem: They way we solve this in the i915 atomic framework is that the various ->fixup calls fill out relevant fields in the drm_crtc_state. I guess we could add an ->atomic_check to bridges, add a bus_flags to drm_crtc_state (or maybe just let everyone patch up adjusted_mode->flags) instead?
Cheers, Daniel
} priv->bridge = bridge;
-- 2.13.5
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hello,
On Monday, 4 September 2017 10:43:55 EEST Daniel Vetter wrote:
On Fri, Sep 01, 2017 at 11:46:29AM +0200, Linus Walleij wrote:
If we cannot find a panel, assume that the output from the PL111 is connected directly to a "dumb" VGA connector, so look up the connector from that bridge.
Signed-off-by: Linus Walleij linus.walleij@linaro.org
This is how the new API is used in the PL111.
drivers/gpu/drm/pl111/Kconfig | 1 + drivers/gpu/drm/pl111/pl111_drv.c | 3 +++ 2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig index e5e2abd66491..82cb3e60ddc8 100644 --- a/drivers/gpu/drm/pl111/Kconfig +++ b/drivers/gpu/drm/pl111/Kconfig @@ -8,6 +8,7 @@ config DRM_PL111
select DRM_GEM_CMA_HELPER select DRM_BRIDGE select DRM_PANEL_BRIDGE
select DRM_DUMB_VGA_DAC
select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE help
Choose this option for DRM support for the PL111 CLCD controller.
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c index f5bc6f160e60..6db423bbd84e 100644 --- a/drivers/gpu/drm/pl111/pl111_drv.c +++ b/drivers/gpu/drm/pl111/pl111_drv.c @@ -67,6 +67,7 @@
#include <drm/drm_of.h> #include <drm/drm_bridge.h> #include <drm/drm_panel.h>
+#include <drm/dumb_vga_dac.h>
#include "pl111_drm.h" #include "pl111_versatile.h"
@@ -128,6 +129,8 @@ static int pl111_modeset_init(struct drm_device *dev)
if (panel) {
priv->panel = panel; priv->connector = panel->connector;
- } else {
priv->connector = drm_dumb_vga_get_connector(bridge);Why do you need to set this? The only code I could find tries to set polarity values from bus_flags, and for VGA I'd assume you want to instead key this off the mode?
Wrt the more general problem: They way we solve this in the i915 atomic framework is that the various ->fixup calls fill out relevant fields in the drm_crtc_state. I guess we could add an ->atomic_check to bridges, add a bus_flags to drm_crtc_state (or maybe just let everyone patch up adjusted_mode->flags) instead?
Furthermore, if we needed to have access to the connector in the display controller driver, this would call for creating the connector there, not in the bridge driver. I've advocated this for a long time, and still haven't given up all hopes of fixing it.
} priv->bridge = bridge;
On Mon, Sep 4, 2017 at 11:51 AM, Laurent Pinchart laurent.pinchart@ideasonboard.com wrote:
On Monday, 4 September 2017 10:43:55 EEST Daniel Vetter wrote:
On Fri, Sep 01, 2017 at 11:46:29AM +0200, Linus Walleij wrote:
Wrt the more general problem: They way we solve this in the i915 atomic framework is that the various ->fixup calls fill out relevant fields in the drm_crtc_state. I guess we could add an ->atomic_check to bridges, add a bus_flags to drm_crtc_state (or maybe just let everyone patch up adjusted_mode->flags) instead?
Furthermore, if we needed to have access to the connector in the display controller driver, this would call for creating the connector there, not in the bridge driver. I've advocated this for a long time, and still haven't given up all hopes of fixing it.
Hm as it is I am trying to adopt to circumstances, so I was just asked to replace the custom connector with the generic panel bridge. (bridge/panel.c)
Then we get the connector out of the generic panel bridge by just dereferencing panel->connector, this is what the PL111 driver is already doing for panels since its inception.
Maybe that is wrong, I don't know :/
But like Eric says, the core problem is that the display driver say enable function needs to know the flags of what it should output. This driver looks at
connector->display_info.bus_flags & DRM_BUS_FLAG_DE_LOW connector->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE
So the bus flags really need to be available somehow. I am doing it by keeping a local reference to the connector in the driver state container.
I tried to dereference the connector from from the pipe at one point, passing NULL to but it didn't work out, it seems like struct drm_simple_display_pipe_funcs .enable() is called before attaching the connector or something but I guess I should look closer.
Yours, Linus Walleij
Daniel Vetter daniel@ffwll.ch writes:
On Fri, Sep 01, 2017 at 11:46:29AM +0200, Linus Walleij wrote:
If we cannot find a panel, assume that the output from the PL111 is connected directly to a "dumb" VGA connector, so look up the connector from that bridge.
Signed-off-by: Linus Walleij linus.walleij@linaro.org
This is how the new API is used in the PL111.
drivers/gpu/drm/pl111/Kconfig | 1 + drivers/gpu/drm/pl111/pl111_drv.c | 3 +++ 2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig index e5e2abd66491..82cb3e60ddc8 100644 --- a/drivers/gpu/drm/pl111/Kconfig +++ b/drivers/gpu/drm/pl111/Kconfig @@ -8,6 +8,7 @@ config DRM_PL111 select DRM_GEM_CMA_HELPER select DRM_BRIDGE select DRM_PANEL_BRIDGE
- select DRM_DUMB_VGA_DAC select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE help Choose this option for DRM support for the PL111 CLCD controller.
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c index f5bc6f160e60..6db423bbd84e 100644 --- a/drivers/gpu/drm/pl111/pl111_drv.c +++ b/drivers/gpu/drm/pl111/pl111_drv.c @@ -67,6 +67,7 @@ #include <drm/drm_of.h> #include <drm/drm_bridge.h> #include <drm/drm_panel.h> +#include <drm/dumb_vga_dac.h>
#include "pl111_drm.h" #include "pl111_versatile.h" @@ -128,6 +129,8 @@ static int pl111_modeset_init(struct drm_device *dev) if (panel) { priv->panel = panel; priv->connector = panel->connector;
- } else {
priv->connector = drm_dumb_vga_get_connector(bridge);Why do you need to set this? The only code I could find tries to set polarity values from bus_flags, and for VGA I'd assume you want to instead key this off the mode?
Wrt the more general problem: They way we solve this in the i915 atomic framework is that the various ->fixup calls fill out relevant fields in the drm_crtc_state. I guess we could add an ->atomic_check to bridges, add a bus_flags to drm_crtc_state (or maybe just let everyone patch up adjusted_mode->flags) instead?
It seems to me like the bus_flags/format should be specific to each link in the display output chain, rather than global to it.
On Tue, Sep 5, 2017 at 9:43 PM, Eric Anholt eric@anholt.net wrote:
Daniel Vetter daniel@ffwll.ch writes:
On Fri, Sep 01, 2017 at 11:46:29AM +0200, Linus Walleij wrote:
If we cannot find a panel, assume that the output from the PL111 is connected directly to a "dumb" VGA connector, so look up the connector from that bridge.
Signed-off-by: Linus Walleij linus.walleij@linaro.org
This is how the new API is used in the PL111.
drivers/gpu/drm/pl111/Kconfig | 1 + drivers/gpu/drm/pl111/pl111_drv.c | 3 +++ 2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig index e5e2abd66491..82cb3e60ddc8 100644 --- a/drivers/gpu/drm/pl111/Kconfig +++ b/drivers/gpu/drm/pl111/Kconfig @@ -8,6 +8,7 @@ config DRM_PL111 select DRM_GEM_CMA_HELPER select DRM_BRIDGE select DRM_PANEL_BRIDGE
- select DRM_DUMB_VGA_DAC select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE help Choose this option for DRM support for the PL111 CLCD controller.
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c index f5bc6f160e60..6db423bbd84e 100644 --- a/drivers/gpu/drm/pl111/pl111_drv.c +++ b/drivers/gpu/drm/pl111/pl111_drv.c @@ -67,6 +67,7 @@ #include <drm/drm_of.h> #include <drm/drm_bridge.h> #include <drm/drm_panel.h> +#include <drm/dumb_vga_dac.h>
#include "pl111_drm.h" #include "pl111_versatile.h" @@ -128,6 +129,8 @@ static int pl111_modeset_init(struct drm_device *dev) if (panel) { priv->panel = panel; priv->connector = panel->connector;
- } else {
priv->connector = drm_dumb_vga_get_connector(bridge);Why do you need to set this? The only code I could find tries to set polarity values from bus_flags, and for VGA I'd assume you want to instead key this off the mode?
Wrt the more general problem: They way we solve this in the i915 atomic framework is that the various ->fixup calls fill out relevant fields in the drm_crtc_state. I guess we could add an ->atomic_check to bridges, add a bus_flags to drm_crtc_state (or maybe just let everyone patch up adjusted_mode->flags) instead?
It seems to me like the bus_flags/format should be specific to each link in the display output chain, rather than global to it.
Yeah that might be the even more general option, to have a drm_bridge_state for this stuff. Thanks to the driver private state support recently added you can add state pointers for any void * (and with a bit of glue even set up type-safe duplicate/get/for_each wrappers). But I'm not clear whether we really need that much generality or not. Currently it's used for the intel shared dpll stuff and the dp mst state, in case someone needs to dig for examples. -Daniel
On Mon, Sep 4, 2017 at 9:43 AM, Daniel Vetter daniel@ffwll.ch wrote:
On Fri, Sep 01, 2017 at 11:46:29AM +0200, Linus Walleij wrote:
If we cannot find a panel, assume that the output from the PL111 is connected directly to a "dumb" VGA connector, so look up the connector from that bridge.
Signed-off-by: Linus Walleij linus.walleij@linaro.org
if (panel) { priv->panel = panel; priv->connector = panel->connector;
} else {priv->connector = drm_dumb_vga_get_connector(bridge);Why do you need to set this?
Essentially because drm_simple_display_pipe_init() require that you specify a connector in the last argument, so I have to get it from somewhere.
The only code I could find tries to set polarity values from bus_flags, and for VGA I'd assume you want to instead key this off the mode?
The usecase is that either a panel or a dumb VGA bridge is used (not both, luckily).
On *some* dumb VGA bridges, the polarity of the clock signal is inverted, i.e. it expects data to be clocked out on the negative edge rather than the positive edge. See: https://lists.freedesktop.org/archives/dri-devel/2017-September/151603.html
And the PL111 can control this in software, that is why it is inspecting the flags on the connector to figure out when to clock things out.
It seems logical to check the connector to figure out what clock edge it wants?
Wrt the more general problem: They way we solve this in the i915 atomic framework is that the various ->fixup calls fill out relevant fields in the drm_crtc_state. I guess we could add an ->atomic_check to bridges, add a bus_flags to drm_crtc_state (or maybe just let everyone patch up adjusted_mode->flags) instead?
Not quite following... I do need fixups to mask some modes off the PL111 but that would be on the driver side.
Yours, Linus Walleij
Den 07.09.2017 14.59, skrev Linus Walleij:
On Mon, Sep 4, 2017 at 9:43 AM, Daniel Vetter daniel@ffwll.ch wrote:
On Fri, Sep 01, 2017 at 11:46:29AM +0200, Linus Walleij wrote:
If we cannot find a panel, assume that the output from the PL111 is connected directly to a "dumb" VGA connector, so look up the connector from that bridge.
Signed-off-by: Linus Walleij linus.walleij@linaro.org if (panel) { priv->panel = panel; priv->connector = panel->connector;
} else {priv->connector = drm_dumb_vga_get_connector(bridge);Why do you need to set this?
Essentially because drm_simple_display_pipe_init() require that you specify a connector in the last argument, so I have to get it from somewhere.
It's optional particularly for the bridge case. From docs:
* If a connector is supplied, the pipe will be coupled with the provided * connector. You may supply a NULL connector when using drm bridges, that * handle connectors themselves (see drm_simple_display_pipe_attach_bridge()).
Noralf.
The only code I could find tries to set polarity values from bus_flags, and for VGA I'd assume you want to instead key this off the mode?
The usecase is that either a panel or a dumb VGA bridge is used (not both, luckily).
On *some* dumb VGA bridges, the polarity of the clock signal is inverted, i.e. it expects data to be clocked out on the negative edge rather than the positive edge. See: https://lists.freedesktop.org/archives/dri-devel/2017-September/151603.html
And the PL111 can control this in software, that is why it is inspecting the flags on the connector to figure out when to clock things out.
It seems logical to check the connector to figure out what clock edge it wants?
Wrt the more general problem: They way we solve this in the i915 atomic framework is that the various ->fixup calls fill out relevant fields in the drm_crtc_state. I guess we could add an ->atomic_check to bridges, add a bus_flags to drm_crtc_state (or maybe just let everyone patch up adjusted_mode->flags) instead?
Not quite following... I do need fixups to mask some modes off the PL111 but that would be on the driver side.
Yours, Linus Walleij _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Sep 7, 2017 at 6:12 PM, Noralf Trønnes noralf@tronnes.org wrote:
Den 07.09.2017 14.59, skrev Linus Walleij:
On Mon, Sep 4, 2017 at 9:43 AM, Daniel Vetter daniel@ffwll.ch wrote:
On Fri, Sep 01, 2017 at 11:46:29AM +0200, Linus Walleij wrote:
If we cannot find a panel, assume that the output from the PL111 is connected directly to a "dumb" VGA connector, so look up the connector from that bridge.
Signed-off-by: Linus Walleij linus.walleij@linaro.org if (panel) { priv->panel = panel; priv->connector = panel->connector;
} else {priv->connector = drm_dumb_vga_get_connector(bridge);Why do you need to set this?
Essentially because drm_simple_display_pipe_init() require that you specify a connector in the last argument, so I have to get it from somewhere.
It's optional particularly for the bridge case. From docs:
- If a connector is supplied, the pipe will be coupled with the provided
- connector. You may supply a NULL connector when using drm bridges, that
- handle connectors themselves (see
drm_simple_display_pipe_attach_bridge()).
Yup, this was the flip side of the work to make bridges useful for the simple pipe helpers. The other one is the helper to set up the bridge I mentioned earlier in this thread. -Daniel
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Eric Anholt -
Laurent Pinchart -
Linus Walleij -
Noralf Trønnes