Re: [RFC V2 0/3] drm/bridge: panel and chaining
On Thu, May 08, 2014 at 02:28:12PM -0400, Rob Clark wrote: On Thu, May 8, 2014 at 7:57 AM, Inki Dae inki@samsung.com wrote: [...] And more thing, we would need to consider image enhancer device placed between crtc and connector/encoder devices. And it'd better to rename drm_hw_block to drm_bridge, and existing drm_bridge relevant codes should be removed. I don't object to changing the name to hw_block or something else. Although to avoid introducing too much confusion in this discussion, for now I am calling it bridge/drm_bridge_funcs/etc for now ;-) FWIW, I think the name drm_bridge is fine. It describes a device that takes a video signal as an input and outputs a video signal, possibly using a different encoding. Thierry pgpkqaQhQhsVm.pgp Description: PGP signature
Re: [RFC V2 0/3] drm/bridge: panel and chaining
On Fri, May 09, 2014 at 09:59:34AM -0400, Rob Clark wrote:
On Fri, May 9, 2014 at 5:08 AM, Andrzej Hajda a.ha...@samsung.com wrote:
[...]
4. And the last thing, it is more about the concept/design. drm_bridge,
drm_hw_block suggests that those interfaces describes the whole device:
bridge, panel, whatever.
hmm, I don't think this is the case. I can easily see things like:
struct foo_panel {
struct drm_panel base;
struct drm_bridge bridge;
...
}
where a panel implementation implements both panel and bridge. In
fact that is kinda what I was encouraging.
For lack of a better entry point into the discussion, let me pick this
example. It seems like in general we all agree that the basic structure
would have to be something like this:
CRTC - encoder - bridge [ - bridge ... ] - panel
Where panel could optionally be a bridge/panel hybrid as Rob pointed
out. I'm not sure if there's panels like this, but I suspect the use
case would be where a panel had built-in controls to modify the image in
some fashion (brightness, saturation, ...). I think those things exist
in DCS for example.
What's missing here, and if I understand correctly what Sean said about
the connector type, what we need to solve is how to wire up the
connector so that it reflects the correct type. So the above would have
to become something like this:
CRTC - encoder - bridge [ - bridge ... ] - panel
connector -^
One of the problems with that approach is that panel is more than just a
video sink. It also controls the panel (and optionally backlight) power.
The standard DRM connector helpers don't work well in that case, because
drm_helper_connector_dpms() forwards changes to the CRTC and encoder,
though that could possibly be solved by wrapping it in a small custom
implementation that also controls the panel. Anyway, that's really just
an implementation detail.
So once a complete chain from encoder to panel has been created, we need
a way to find the appropriate connector type. Perhaps to achieve that it
would be useful to instantiate the connector by the bridge that's
connected to the panel. So essentially something like this:
struct drm_bridge {
/* to allow chaining */
struct drm_bridge *next;
/* for bridges directly connected to a panel or monitor */
struct drm_connector *connector;
/* for bridges directly connected to a panel */
struct drm_panel *panel;
...
};
To make this work in a generic way, I think we're missing one bridge
instance. Currently the bridge in an encoder is completely optional, so
if there is no bridge in the system this needs to be special cased. One
way to unify this would be to make drm_encoder implement drm_bridge, or
an alternative would be to make drm_panel implement a bridge. Both can
possibly be dummies stubbed out with simple helpers.
I wonder if this would have any consequences on Dave's DP MST work,
since presumably an MST hub could be considered a bridge. In that case I
guess there would need to be support for multiple next bridges,
perhaps a simple doubly-linked list instead of a next pointer. The first
bridge would then be used to model the hub's input and child bridges
would be used to model each
of the outputs.
Thierry
pgpFQ9UQJFyeK.pgp
Description: PGP signature
Re: [RFC V2 0/3] drm/bridge: panel and chaining
On Thu, May 15, 2014 at 12:32:58PM +0200, Thierry Reding wrote:
On Fri, May 09, 2014 at 09:59:34AM -0400, Rob Clark wrote:
On Fri, May 9, 2014 at 5:08 AM, Andrzej Hajda a.ha...@samsung.com wrote:
[...]
4. And the last thing, it is more about the concept/design. drm_bridge,
drm_hw_block suggests that those interfaces describes the whole device:
bridge, panel, whatever.
hmm, I don't think this is the case. I can easily see things like:
struct foo_panel {
struct drm_panel base;
struct drm_bridge bridge;
...
}
where a panel implementation implements both panel and bridge. In
fact that is kinda what I was encouraging.
For lack of a better entry point into the discussion, let me pick this
example. It seems like in general we all agree that the basic structure
would have to be something like this:
CRTC - encoder - bridge [ - bridge ... ] - panel
Where panel could optionally be a bridge/panel hybrid as Rob pointed
out. I'm not sure if there's panels like this, but I suspect the use
case would be where a panel had built-in controls to modify the image in
some fashion (brightness, saturation, ...). I think those things exist
in DCS for example.
What's missing here, and if I understand correctly what Sean said about
the connector type, what we need to solve is how to wire up the
connector so that it reflects the correct type. So the above would have
to become something like this:
CRTC - encoder - bridge [ - bridge ... ] - panel
connector -^
This is kinda always how I've thought it should play out: The last item in
the chain actually implements the drm connector, everyone else just
ignores it.
One of the problems with that approach is that panel is more than just a
video sink. It also controls the panel (and optionally backlight) power.
The standard DRM connector helpers don't work well in that case, because
drm_helper_connector_dpms() forwards changes to the CRTC and encoder,
though that could possibly be solved by wrapping it in a small custom
implementation that also controls the panel. Anyway, that's really just
an implementation detail.
I don't see an issue with the dpms really. At worst the overall kms driver
(which provides the crtc and encoder implementation) needs to pass a
suitable dpms implementation for the drm_bridge to use in the connector.
Or we could just move this vfunc into the core kms funtion table since
everyone uses the same (well except i915, but that's a different story).
dpms changes would then still be driven through the crtc - encoder -
bridge ... chain.
So once a complete chain from encoder to panel has been created, we need
a way to find the appropriate connector type. Perhaps to achieve that it
would be useful to instantiate the connector by the bridge that's
connected to the panel. So essentially something like this:
struct drm_bridge {
/* to allow chaining */
struct drm_bridge *next;
/* for bridges directly connected to a panel or monitor */
struct drm_connector *connector;
/* for bridges directly connected to a panel */
struct drm_panel *panel;
Imo these two shouldn't be here, but instead should be embedded into the
overall structure the drm_bridge driver is using.
...
};
To make this work in a generic way, I think we're missing one bridge
instance. Currently the bridge in an encoder is completely optional, so
if there is no bridge in the system this needs to be special cased. One
way to unify this would be to make drm_encoder implement drm_bridge, or
an alternative would be to make drm_panel implement a bridge. Both can
possibly be dummies stubbed out with simple helpers.
Yeah, imo if the panel isn't just a dumb thing but needs to actively take
part in the modeset dance, it simply needs to implement itself as a
drm_bridge+panel combo and do the magic in the various hooks provided.
I wonder if this would have any consequences on Dave's DP MST work,
since presumably an MST hub could be considered a bridge. In that case I
guess there would need to be support for multiple next bridges,
perhaps a simple doubly-linked list instead of a next pointer. The first
bridge would then be used to model the hub's input and child bridges
would be used to model each
of the outputs.
DP is standardize. No need to wrestle the complexity through a drm_bridge,
since code reuse anywhere else (non-DP) will be know to be zero. And for
all the guys implementing a DP output can simply use the helpers. So I
don't see an upside of this idea.
Imo we should restrict drm_bridge to all the crazy transcoder chips/blocks
commonly found on SoCs which all do non-standard stuff and where we
actually have an established or anticipated need for sharing.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48
Re: [RFC V2 0/3] drm/bridge: panel and chaining
On Thu, May 15, 2014 at 6:04 PM, Daniel Vetter dan...@ffwll.ch wrote:
On Thu, May 15, 2014 at 12:32:58PM +0200, Thierry Reding wrote:
On Fri, May 09, 2014 at 09:59:34AM -0400, Rob Clark wrote:
On Fri, May 9, 2014 at 5:08 AM, Andrzej Hajda a.ha...@samsung.com wrote:
[...]
4. And the last thing, it is more about the concept/design. drm_bridge,
drm_hw_block suggests that those interfaces describes the whole device:
bridge, panel, whatever.
hmm, I don't think this is the case. I can easily see things like:
struct foo_panel {
struct drm_panel base;
struct drm_bridge bridge;
...
}
where a panel implementation implements both panel and bridge. In
fact that is kinda what I was encouraging.
For lack of a better entry point into the discussion, let me pick this
example. It seems like in general we all agree that the basic structure
would have to be something like this:
CRTC - encoder - bridge [ - bridge ... ] - panel
Where panel could optionally be a bridge/panel hybrid as Rob pointed
out. I'm not sure if there's panels like this, but I suspect the use
case would be where a panel had built-in controls to modify the image in
some fashion (brightness, saturation, ...). I think those things exist
in DCS for example.
What's missing here, and if I understand correctly what Sean said about
the connector type, what we need to solve is how to wire up the
connector so that it reflects the correct type. So the above would have
to become something like this:
CRTC - encoder - bridge [ - bridge ... ] - panel
connector -^
This is kinda always how I've thought it should play out: The last item in
the chain actually implements the drm connector, everyone else just
ignores it.
The last bridge or the panel?
One of the problems with that approach is that panel is more than just a
video sink. It also controls the panel (and optionally backlight) power.
The standard DRM connector helpers don't work well in that case, because
drm_helper_connector_dpms() forwards changes to the CRTC and encoder,
though that could possibly be solved by wrapping it in a small custom
implementation that also controls the panel. Anyway, that's really just
an implementation detail.
Why did this discussion come up? Are we going to call panel controls
from a common layer?
I don't see an issue with the dpms really. At worst the overall kms driver
(which provides the crtc and encoder implementation) needs to pass a
suitable dpms implementation for the drm_bridge to use in the connector.
Or we could just move this vfunc into the core kms funtion table since
everyone uses the same (well except i915, but that's a different story).
dpms changes would then still be driven through the crtc - encoder -
bridge ... chain.
So once a complete chain from encoder to panel has been created, we need
a way to find the appropriate connector type. Perhaps to achieve that it
would be useful to instantiate the connector by the bridge that's
connected to the panel. So essentially something like this:
struct drm_bridge {
/* to allow chaining */
struct drm_bridge *next;
/* for bridges directly connected to a panel or monitor */
struct drm_connector *connector;
/* for bridges directly connected to a panel */
struct drm_panel *panel;
Imo these two shouldn't be here, but instead should be embedded into the
overall structure the drm_bridge driver is using.
...
};
To make this work in a generic way, I think we're missing one bridge
instance. Currently the bridge in an encoder is completely optional, so
if there is no bridge in the system this needs to be special cased. One
way to unify this would be to make drm_encoder implement drm_bridge, or
an alternative would be to make drm_panel implement a bridge. Both can
possibly be dummies stubbed out with simple helpers.
Yeah, imo if the panel isn't just a dumb thing but needs to actively take
part in the modeset dance, it simply needs to implement itself as a
drm_bridge+panel combo and do the magic in the various hooks provided.
And, what does one mean when a real panel should implement both
drm_panel and drm_bridge?
who will call drm_bridge-funcs for the panel?, and
who will call drm_panel-funcs for the panel?
Can someone please elaborate this with existing implementation details
for bridge and panel? I am really getting confused.
I wonder if this would have any consequences on Dave's DP MST work,
since presumably an MST hub could be considered a bridge. In that case I
guess there would need to be support for multiple next bridges,
perhaps a simple doubly-linked list instead of a next pointer. The first
bridge would then be used to model the hub's input and child bridges
would be used to model each
of the outputs.
DP is standardize. No need to
Re: [RFC V2 0/3] drm/bridge: panel and chaining
On 05/12/2014 02:45 PM, Rob Clark wrote:
On Mon, May 12, 2014 at 3:06 AM, Andrzej Hajda a.ha...@samsung.com wrote:
On 05/09/2014 05:05 PM, Ajay kumar wrote:
On Fri, May 9, 2014 at 7:29 PM, Rob Clark robdcl...@gmail.com wrote:
On Fri, May 9, 2014 at 5:08 AM, Andrzej Hajda a.ha...@samsung.com wrote:
On 05/08/2014 08:24 PM, Rob Clark wrote:
On Thu, May 8, 2014 at 2:41 AM, Andrzej Hajda a.ha...@samsung.com
wrote:
On 05/05/2014 09:52 PM, Ajay Kumar wrote:
This patchset is based on exynos-drm-next-todo branch of Inki Dae's
tree at:
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
I have just put up Rob's and Sean's idea of chaining up the bridges
in code, and have implemented basic panel controls as a chained bridge.
This works well with ptn3460 bridge chip on exynos5250-snow board.
Still need to make use of standard list calls and figure out proper way
of deleting the bridge chain. So, this is just a rough version.
As I understand this patchset tries to solve two things:
1. Implement panel as drm_bridge, to ease support for hardware chains:
Crtc - Encoder - Bridge - Panel
2. Add support to drm_bridge chaining, to allow software chains:
drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel
It is done using Russian doll schema, ops from the bridge calls the same
ops from the next bridge and the next bridge ops can do the same.
This schema means that all the bridges including the last one are seen
from the drm core point of view as a one big drm_bridge. Additionally in
this particular case, the first bridge (ptn3460) implements connector
so it is hard to guess what is the location of the 2nd bridge in video
stream chain, sometimes it is after the connector, sometimes before.
All this is quite confusing.
But if you look at the bridge from upstream video interface point of
view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its
video input side acts as a panel. On the output side it expects a panel,
lvds panel in this case.
tbh, this is mostly about what we call it. Perhaps bridge isn't the
best name.. I wouldn't object to changing it.
But my thinking was to leave in drm_panel_funcs things that are just
needed by the connector (get_modes().. and maybe some day we need
detect/etc). Then leave everything else in drm_bridge_funcs. A panel
could (if needed) implement both interfaces.
That is basically the same as what you are proposing, but without
renaming bridge to panel ;-)
Good to hear that. However there are points which are not clear for me.
But first lets clarify names, I will use panel and bridge words
to describe the hardware, and drm_panel, drm_bridge to describe the
software interfaces.
What bothers me:
1. You want to leave connector related callbacks in drm_panel and
the rest in drm_bridge. In case of ptn3460 it does not work, ptn3460
must implement connector internally because of this limitation. I guess
it is quite typical bridge. This problem does not exists in case
of using drm_panel for ptn3460.
2. drm_bridge is attached to the encoder, this and the callback order
suggests the video data flow should be as below:
drm_crtc - drm_encoder [- drm_bridge] - drm_connector [- drm_panel]
ptn3460 implements drm_bridge and drm_connector so it suggests its
drm_bridge should be the last one, so there should be no place to add
lvds panel implemented as a drm_bridge after it, as it is done in this
patchset.
Additionally it clearly shows that there should be two categories of
drm_bridges - non-terminal and terminal.
3. drm_dev uses all-or-nothing approach, ie. it will start only when all
its components are up. It simplifies synchronization but is quite
fragile - the whole drm will be down due to error in some of its
components.
For this reason I prefer drm_panel as it is not real drm component
it can be attached/detached to/from drm_connector anytime. I am not
really sure but drm_bridge does not allow for that.
So I do think we need to stick to this all-or-nothing approach for
anything that is visible to userspace
(drm_{plane,crtc,encoder,connector}). We don't currently have a way
to hotplug those so I don't see a real smooth upgrade path to add
that in a backwards compatible way that won't cause problems with old
userspace.
But, that said, we have more flexibility with things not visible to
userspace (drm_{panel,bridge}). I'm not sure how much we want to
allow things to be completely dynamic (we already have some hard
enough locking fun). But proposals/rfcs/etc welcome.
I guess I'm not completely familiar w/ ptn3460, but the fact that it
needs to implement drm_connector makes me a bit suspicious. Seems
like a symptom of missing things in drm_panel_funcs. It would be
better to always create the connector statically, and just have
_detect() - disconnected if panel==NULL.
ptn3460 has been implemented using drm_bridge and drm_connector, not by
me, to be clear :)
sure, and
Re: [RFC V2 0/3] drm/bridge: panel and chaining
On 05/12/2014 06:00 PM, Sean Paul wrote:
On Mon, May 12, 2014 at 3:06 AM, Andrzej Hajda a.ha...@samsung.com wrote:
On 05/09/2014 05:05 PM, Ajay kumar wrote:
On Fri, May 9, 2014 at 7:29 PM, Rob Clark robdcl...@gmail.com wrote:
On Fri, May 9, 2014 at 5:08 AM, Andrzej Hajda a.ha...@samsung.com wrote:
On 05/08/2014 08:24 PM, Rob Clark wrote:
On Thu, May 8, 2014 at 2:41 AM, Andrzej Hajda a.ha...@samsung.com
wrote:
On 05/05/2014 09:52 PM, Ajay Kumar wrote:
This patchset is based on exynos-drm-next-todo branch of Inki Dae's
tree at:
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
I have just put up Rob's and Sean's idea of chaining up the bridges
in code, and have implemented basic panel controls as a chained bridge.
This works well with ptn3460 bridge chip on exynos5250-snow board.
Still need to make use of standard list calls and figure out proper way
of deleting the bridge chain. So, this is just a rough version.
As I understand this patchset tries to solve two things:
1. Implement panel as drm_bridge, to ease support for hardware chains:
Crtc - Encoder - Bridge - Panel
2. Add support to drm_bridge chaining, to allow software chains:
drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel
It is done using Russian doll schema, ops from the bridge calls the same
ops from the next bridge and the next bridge ops can do the same.
This schema means that all the bridges including the last one are seen
from the drm core point of view as a one big drm_bridge. Additionally in
this particular case, the first bridge (ptn3460) implements connector
so it is hard to guess what is the location of the 2nd bridge in video
stream chain, sometimes it is after the connector, sometimes before.
All this is quite confusing.
But if you look at the bridge from upstream video interface point of
view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its
video input side acts as a panel. On the output side it expects a panel,
lvds panel in this case.
tbh, this is mostly about what we call it. Perhaps bridge isn't the
best name.. I wouldn't object to changing it.
But my thinking was to leave in drm_panel_funcs things that are just
needed by the connector (get_modes().. and maybe some day we need
detect/etc). Then leave everything else in drm_bridge_funcs. A panel
could (if needed) implement both interfaces.
That is basically the same as what you are proposing, but without
renaming bridge to panel ;-)
Good to hear that. However there are points which are not clear for me.
But first lets clarify names, I will use panel and bridge words
to describe the hardware, and drm_panel, drm_bridge to describe the
software interfaces.
What bothers me:
1. You want to leave connector related callbacks in drm_panel and
the rest in drm_bridge. In case of ptn3460 it does not work, ptn3460
must implement connector internally because of this limitation. I guess
it is quite typical bridge. This problem does not exists in case
of using drm_panel for ptn3460.
2. drm_bridge is attached to the encoder, this and the callback order
suggests the video data flow should be as below:
drm_crtc - drm_encoder [- drm_bridge] - drm_connector [- drm_panel]
ptn3460 implements drm_bridge and drm_connector so it suggests its
drm_bridge should be the last one, so there should be no place to add
lvds panel implemented as a drm_bridge after it, as it is done in this
patchset.
Additionally it clearly shows that there should be two categories of
drm_bridges - non-terminal and terminal.
3. drm_dev uses all-or-nothing approach, ie. it will start only when all
its components are up. It simplifies synchronization but is quite
fragile - the whole drm will be down due to error in some of its
components.
For this reason I prefer drm_panel as it is not real drm component
it can be attached/detached to/from drm_connector anytime. I am not
really sure but drm_bridge does not allow for that.
So I do think we need to stick to this all-or-nothing approach for
anything that is visible to userspace
(drm_{plane,crtc,encoder,connector}). We don't currently have a way
to hotplug those so I don't see a real smooth upgrade path to add
that in a backwards compatible way that won't cause problems with old
userspace.
But, that said, we have more flexibility with things not visible to
userspace (drm_{panel,bridge}). I'm not sure how much we want to
allow things to be completely dynamic (we already have some hard
enough locking fun). But proposals/rfcs/etc welcome.
I guess I'm not completely familiar w/ ptn3460, but the fact that it
needs to implement drm_connector makes me a bit suspicious. Seems
like a symptom of missing things in drm_panel_funcs. It would be
better to always create the connector statically, and just have
_detect() - disconnected if panel==NULL.
ptn3460 has been implemented using drm_bridge and drm_connector, not by
me, to be clear :)
Right,
Re: [RFC V2 0/3] drm/bridge: panel and chaining
On 05/09/2014 05:05 PM, Ajay kumar wrote:
On Fri, May 9, 2014 at 7:29 PM, Rob Clark robdcl...@gmail.com wrote:
On Fri, May 9, 2014 at 5:08 AM, Andrzej Hajda a.ha...@samsung.com wrote:
On 05/08/2014 08:24 PM, Rob Clark wrote:
On Thu, May 8, 2014 at 2:41 AM, Andrzej Hajda a.ha...@samsung.com wrote:
On 05/05/2014 09:52 PM, Ajay Kumar wrote:
This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree
at:
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
I have just put up Rob's and Sean's idea of chaining up the bridges
in code, and have implemented basic panel controls as a chained bridge.
This works well with ptn3460 bridge chip on exynos5250-snow board.
Still need to make use of standard list calls and figure out proper way
of deleting the bridge chain. So, this is just a rough version.
As I understand this patchset tries to solve two things:
1. Implement panel as drm_bridge, to ease support for hardware chains:
Crtc - Encoder - Bridge - Panel
2. Add support to drm_bridge chaining, to allow software chains:
drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel
It is done using Russian doll schema, ops from the bridge calls the same
ops from the next bridge and the next bridge ops can do the same.
This schema means that all the bridges including the last one are seen
from the drm core point of view as a one big drm_bridge. Additionally in
this particular case, the first bridge (ptn3460) implements connector
so it is hard to guess what is the location of the 2nd bridge in video
stream chain, sometimes it is after the connector, sometimes before.
All this is quite confusing.
But if you look at the bridge from upstream video interface point of
view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its
video input side acts as a panel. On the output side it expects a panel,
lvds panel in this case.
tbh, this is mostly about what we call it. Perhaps bridge isn't the
best name.. I wouldn't object to changing it.
But my thinking was to leave in drm_panel_funcs things that are just
needed by the connector (get_modes().. and maybe some day we need
detect/etc). Then leave everything else in drm_bridge_funcs. A panel
could (if needed) implement both interfaces.
That is basically the same as what you are proposing, but without
renaming bridge to panel ;-)
Good to hear that. However there are points which are not clear for me.
But first lets clarify names, I will use panel and bridge words
to describe the hardware, and drm_panel, drm_bridge to describe the
software interfaces.
What bothers me:
1. You want to leave connector related callbacks in drm_panel and
the rest in drm_bridge. In case of ptn3460 it does not work, ptn3460
must implement connector internally because of this limitation. I guess
it is quite typical bridge. This problem does not exists in case
of using drm_panel for ptn3460.
2. drm_bridge is attached to the encoder, this and the callback order
suggests the video data flow should be as below:
drm_crtc - drm_encoder [- drm_bridge] - drm_connector [- drm_panel]
ptn3460 implements drm_bridge and drm_connector so it suggests its
drm_bridge should be the last one, so there should be no place to add
lvds panel implemented as a drm_bridge after it, as it is done in this
patchset.
Additionally it clearly shows that there should be two categories of
drm_bridges - non-terminal and terminal.
3. drm_dev uses all-or-nothing approach, ie. it will start only when all
its components are up. It simplifies synchronization but is quite
fragile - the whole drm will be down due to error in some of its components.
For this reason I prefer drm_panel as it is not real drm component
it can be attached/detached to/from drm_connector anytime. I am not
really sure but drm_bridge does not allow for that.
So I do think we need to stick to this all-or-nothing approach for
anything that is visible to userspace
(drm_{plane,crtc,encoder,connector}). We don't currently have a way
to hotplug those so I don't see a real smooth upgrade path to add
that in a backwards compatible way that won't cause problems with old
userspace.
But, that said, we have more flexibility with things not visible to
userspace (drm_{panel,bridge}). I'm not sure how much we want to
allow things to be completely dynamic (we already have some hard
enough locking fun). But proposals/rfcs/etc welcome.
I guess I'm not completely familiar w/ ptn3460, but the fact that it
needs to implement drm_connector makes me a bit suspicious. Seems
like a symptom of missing things in drm_panel_funcs. It would be
better to always create the connector statically, and just have
_detect() - disconnected if panel==NULL.
ptn3460 has been implemented using drm_bridge and drm_connector, not by
me, to be clear :)
And to make it more clear from what I see ptn3460 exposes following ops:
- pre_enable (via drm_bridge).
- disable (via drm_bridge),
-
Re: [RFC V2 0/3] drm/bridge: panel and chaining
On Mon, May 12, 2014 at 3:06 AM, Andrzej Hajda a.ha...@samsung.com wrote:
On 05/09/2014 05:05 PM, Ajay kumar wrote:
On Fri, May 9, 2014 at 7:29 PM, Rob Clark robdcl...@gmail.com wrote:
On Fri, May 9, 2014 at 5:08 AM, Andrzej Hajda a.ha...@samsung.com wrote:
On 05/08/2014 08:24 PM, Rob Clark wrote:
On Thu, May 8, 2014 at 2:41 AM, Andrzej Hajda a.ha...@samsung.com wrote:
On 05/05/2014 09:52 PM, Ajay Kumar wrote:
This patchset is based on exynos-drm-next-todo branch of Inki Dae's
tree at:
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
I have just put up Rob's and Sean's idea of chaining up the bridges
in code, and have implemented basic panel controls as a chained bridge.
This works well with ptn3460 bridge chip on exynos5250-snow board.
Still need to make use of standard list calls and figure out proper way
of deleting the bridge chain. So, this is just a rough version.
As I understand this patchset tries to solve two things:
1. Implement panel as drm_bridge, to ease support for hardware chains:
Crtc - Encoder - Bridge - Panel
2. Add support to drm_bridge chaining, to allow software chains:
drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel
It is done using Russian doll schema, ops from the bridge calls the same
ops from the next bridge and the next bridge ops can do the same.
This schema means that all the bridges including the last one are seen
from the drm core point of view as a one big drm_bridge. Additionally in
this particular case, the first bridge (ptn3460) implements connector
so it is hard to guess what is the location of the 2nd bridge in video
stream chain, sometimes it is after the connector, sometimes before.
All this is quite confusing.
But if you look at the bridge from upstream video interface point of
view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its
video input side acts as a panel. On the output side it expects a panel,
lvds panel in this case.
tbh, this is mostly about what we call it. Perhaps bridge isn't the
best name.. I wouldn't object to changing it.
But my thinking was to leave in drm_panel_funcs things that are just
needed by the connector (get_modes().. and maybe some day we need
detect/etc). Then leave everything else in drm_bridge_funcs. A panel
could (if needed) implement both interfaces.
That is basically the same as what you are proposing, but without
renaming bridge to panel ;-)
Good to hear that. However there are points which are not clear for me.
But first lets clarify names, I will use panel and bridge words
to describe the hardware, and drm_panel, drm_bridge to describe the
software interfaces.
What bothers me:
1. You want to leave connector related callbacks in drm_panel and
the rest in drm_bridge. In case of ptn3460 it does not work, ptn3460
must implement connector internally because of this limitation. I guess
it is quite typical bridge. This problem does not exists in case
of using drm_panel for ptn3460.
2. drm_bridge is attached to the encoder, this and the callback order
suggests the video data flow should be as below:
drm_crtc - drm_encoder [- drm_bridge] - drm_connector [- drm_panel]
ptn3460 implements drm_bridge and drm_connector so it suggests its
drm_bridge should be the last one, so there should be no place to add
lvds panel implemented as a drm_bridge after it, as it is done in this
patchset.
Additionally it clearly shows that there should be two categories of
drm_bridges - non-terminal and terminal.
3. drm_dev uses all-or-nothing approach, ie. it will start only when all
its components are up. It simplifies synchronization but is quite
fragile - the whole drm will be down due to error in some of its
components.
For this reason I prefer drm_panel as it is not real drm component
it can be attached/detached to/from drm_connector anytime. I am not
really sure but drm_bridge does not allow for that.
So I do think we need to stick to this all-or-nothing approach for
anything that is visible to userspace
(drm_{plane,crtc,encoder,connector}). We don't currently have a way
to hotplug those so I don't see a real smooth upgrade path to add
that in a backwards compatible way that won't cause problems with old
userspace.
But, that said, we have more flexibility with things not visible to
userspace (drm_{panel,bridge}). I'm not sure how much we want to
allow things to be completely dynamic (we already have some hard
enough locking fun). But proposals/rfcs/etc welcome.
I guess I'm not completely familiar w/ ptn3460, but the fact that it
needs to implement drm_connector makes me a bit suspicious. Seems
like a symptom of missing things in drm_panel_funcs. It would be
better to always create the connector statically, and just have
_detect() - disconnected if panel==NULL.
ptn3460 has been implemented using drm_bridge and drm_connector, not by
me, to be clear :)
sure, and afaiu it was adapted from a pre-bridge
Re: [RFC V2 0/3] drm/bridge: panel and chaining
On 05/08/2014 08:24 PM, Rob Clark wrote: On Thu, May 8, 2014 at 2:41 AM, Andrzej Hajda a.ha...@samsung.com wrote: On 05/05/2014 09:52 PM, Ajay Kumar wrote: This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git I have just put up Rob's and Sean's idea of chaining up the bridges in code, and have implemented basic panel controls as a chained bridge. This works well with ptn3460 bridge chip on exynos5250-snow board. Still need to make use of standard list calls and figure out proper way of deleting the bridge chain. So, this is just a rough version. As I understand this patchset tries to solve two things: 1. Implement panel as drm_bridge, to ease support for hardware chains: Crtc - Encoder - Bridge - Panel 2. Add support to drm_bridge chaining, to allow software chains: drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel It is done using Russian doll schema, ops from the bridge calls the same ops from the next bridge and the next bridge ops can do the same. This schema means that all the bridges including the last one are seen from the drm core point of view as a one big drm_bridge. Additionally in this particular case, the first bridge (ptn3460) implements connector so it is hard to guess what is the location of the 2nd bridge in video stream chain, sometimes it is after the connector, sometimes before. All this is quite confusing. But if you look at the bridge from upstream video interface point of view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its video input side acts as a panel. On the output side it expects a panel, lvds panel in this case. tbh, this is mostly about what we call it. Perhaps bridge isn't the best name.. I wouldn't object to changing it. But my thinking was to leave in drm_panel_funcs things that are just needed by the connector (get_modes().. and maybe some day we need detect/etc). Then leave everything else in drm_bridge_funcs. A panel could (if needed) implement both interfaces. That is basically the same as what you are proposing, but without renaming bridge to panel ;-) Good to hear that. However there are points which are not clear for me. But first lets clarify names, I will use panel and bridge words to describe the hardware, and drm_panel, drm_bridge to describe the software interfaces. What bothers me: 1. You want to leave connector related callbacks in drm_panel and the rest in drm_bridge. In case of ptn3460 it does not work, ptn3460 must implement connector internally because of this limitation. I guess it is quite typical bridge. This problem does not exists in case of using drm_panel for ptn3460. 2. drm_bridge is attached to the encoder, this and the callback order suggests the video data flow should be as below: drm_crtc - drm_encoder [- drm_bridge] - drm_connector [- drm_panel] ptn3460 implements drm_bridge and drm_connector so it suggests its drm_bridge should be the last one, so there should be no place to add lvds panel implemented as a drm_bridge after it, as it is done in this patchset. Additionally it clearly shows that there should be two categories of drm_bridges - non-terminal and terminal. 3. drm_dev uses all-or-nothing approach, ie. it will start only when all its components are up. It simplifies synchronization but is quite fragile - the whole drm will be down due to error in some of its components. For this reason I prefer drm_panel as it is not real drm component it can be attached/detached to/from drm_connector anytime. I am not really sure but drm_bridge does not allow for that. Real life example to show importance of it: I have a phone with MIPI-DSI panel and HDMI. Due to initialization issues HDMI bridge driver sometimes fails during probe and the drmdev do not start. Of course this is development stage so I have serial console I can diagnose the problem, disable HDMI, fix the problem, etc... But what happens in case of end-user. He will see black screen - bricked phone. In case the bridge will be implemented using drm_panel he will have working phone with broken HDMI, much better. 4. And the last thing, it is more about the concept/design. drm_bridge, drm_hw_block suggests that those interfaces describes the whole device: bridge, panel, whatever. In my approach I have an interface to describe only one video input port of the device. And drm_panel is in fact misleading name, drm_sink may be better. So real panel would implement drm_sink interface. Bridge would implement drm_sink interface and it will request other drm_sink interface, to interact with the panel which is after it. This approach seems to me more flexible. Beside things I have described above it will allow to implement also more complicated devices, dsi hubs, video mixers, etc. Regards Andrzej BR, -R So why not implement ptn3460 bridge as drm_panel which internally uses another drm_panel. With this approach
Re: [RFC V2 0/3] drm/bridge: panel and chaining
On Fri, May 9, 2014 at 5:08 AM, Andrzej Hajda a.ha...@samsung.com wrote:
On 05/08/2014 08:24 PM, Rob Clark wrote:
On Thu, May 8, 2014 at 2:41 AM, Andrzej Hajda a.ha...@samsung.com wrote:
On 05/05/2014 09:52 PM, Ajay Kumar wrote:
This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree
at:
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
I have just put up Rob's and Sean's idea of chaining up the bridges
in code, and have implemented basic panel controls as a chained bridge.
This works well with ptn3460 bridge chip on exynos5250-snow board.
Still need to make use of standard list calls and figure out proper way
of deleting the bridge chain. So, this is just a rough version.
As I understand this patchset tries to solve two things:
1. Implement panel as drm_bridge, to ease support for hardware chains:
Crtc - Encoder - Bridge - Panel
2. Add support to drm_bridge chaining, to allow software chains:
drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel
It is done using Russian doll schema, ops from the bridge calls the same
ops from the next bridge and the next bridge ops can do the same.
This schema means that all the bridges including the last one are seen
from the drm core point of view as a one big drm_bridge. Additionally in
this particular case, the first bridge (ptn3460) implements connector
so it is hard to guess what is the location of the 2nd bridge in video
stream chain, sometimes it is after the connector, sometimes before.
All this is quite confusing.
But if you look at the bridge from upstream video interface point of
view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its
video input side acts as a panel. On the output side it expects a panel,
lvds panel in this case.
tbh, this is mostly about what we call it. Perhaps bridge isn't the
best name.. I wouldn't object to changing it.
But my thinking was to leave in drm_panel_funcs things that are just
needed by the connector (get_modes().. and maybe some day we need
detect/etc). Then leave everything else in drm_bridge_funcs. A panel
could (if needed) implement both interfaces.
That is basically the same as what you are proposing, but without
renaming bridge to panel ;-)
Good to hear that. However there are points which are not clear for me.
But first lets clarify names, I will use panel and bridge words
to describe the hardware, and drm_panel, drm_bridge to describe the
software interfaces.
What bothers me:
1. You want to leave connector related callbacks in drm_panel and
the rest in drm_bridge. In case of ptn3460 it does not work, ptn3460
must implement connector internally because of this limitation. I guess
it is quite typical bridge. This problem does not exists in case
of using drm_panel for ptn3460.
2. drm_bridge is attached to the encoder, this and the callback order
suggests the video data flow should be as below:
drm_crtc - drm_encoder [- drm_bridge] - drm_connector [- drm_panel]
ptn3460 implements drm_bridge and drm_connector so it suggests its
drm_bridge should be the last one, so there should be no place to add
lvds panel implemented as a drm_bridge after it, as it is done in this
patchset.
Additionally it clearly shows that there should be two categories of
drm_bridges - non-terminal and terminal.
3. drm_dev uses all-or-nothing approach, ie. it will start only when all
its components are up. It simplifies synchronization but is quite
fragile - the whole drm will be down due to error in some of its components.
For this reason I prefer drm_panel as it is not real drm component
it can be attached/detached to/from drm_connector anytime. I am not
really sure but drm_bridge does not allow for that.
So I do think we need to stick to this all-or-nothing approach for
anything that is visible to userspace
(drm_{plane,crtc,encoder,connector}). We don't currently have a way
to hotplug those so I don't see a real smooth upgrade path to add
that in a backwards compatible way that won't cause problems with old
userspace.
But, that said, we have more flexibility with things not visible to
userspace (drm_{panel,bridge}). I'm not sure how much we want to
allow things to be completely dynamic (we already have some hard
enough locking fun). But proposals/rfcs/etc welcome.
I guess I'm not completely familiar w/ ptn3460, but the fact that it
needs to implement drm_connector makes me a bit suspicious. Seems
like a symptom of missing things in drm_panel_funcs. It would be
better to always create the connector statically, and just have
_detect() - disconnected if panel==NULL.
Real life example to show importance of it: I have a phone with MIPI-DSI
panel and HDMI. Due to initialization issues HDMI bridge driver
sometimes fails during probe and the drmdev do not start. Of course this
is development stage so I have serial console I can diagnose the
problem, disable HDMI, fix the problem, etc...
But what happens in
Re: [RFC V2 0/3] drm/bridge: panel and chaining
On Fri, May 9, 2014 at 7:29 PM, Rob Clark robdcl...@gmail.com wrote:
On Fri, May 9, 2014 at 5:08 AM, Andrzej Hajda a.ha...@samsung.com wrote:
On 05/08/2014 08:24 PM, Rob Clark wrote:
On Thu, May 8, 2014 at 2:41 AM, Andrzej Hajda a.ha...@samsung.com wrote:
On 05/05/2014 09:52 PM, Ajay Kumar wrote:
This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree
at:
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
I have just put up Rob's and Sean's idea of chaining up the bridges
in code, and have implemented basic panel controls as a chained bridge.
This works well with ptn3460 bridge chip on exynos5250-snow board.
Still need to make use of standard list calls and figure out proper way
of deleting the bridge chain. So, this is just a rough version.
As I understand this patchset tries to solve two things:
1. Implement panel as drm_bridge, to ease support for hardware chains:
Crtc - Encoder - Bridge - Panel
2. Add support to drm_bridge chaining, to allow software chains:
drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel
It is done using Russian doll schema, ops from the bridge calls the same
ops from the next bridge and the next bridge ops can do the same.
This schema means that all the bridges including the last one are seen
from the drm core point of view as a one big drm_bridge. Additionally in
this particular case, the first bridge (ptn3460) implements connector
so it is hard to guess what is the location of the 2nd bridge in video
stream chain, sometimes it is after the connector, sometimes before.
All this is quite confusing.
But if you look at the bridge from upstream video interface point of
view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its
video input side acts as a panel. On the output side it expects a panel,
lvds panel in this case.
tbh, this is mostly about what we call it. Perhaps bridge isn't the
best name.. I wouldn't object to changing it.
But my thinking was to leave in drm_panel_funcs things that are just
needed by the connector (get_modes().. and maybe some day we need
detect/etc). Then leave everything else in drm_bridge_funcs. A panel
could (if needed) implement both interfaces.
That is basically the same as what you are proposing, but without
renaming bridge to panel ;-)
Good to hear that. However there are points which are not clear for me.
But first lets clarify names, I will use panel and bridge words
to describe the hardware, and drm_panel, drm_bridge to describe the
software interfaces.
What bothers me:
1. You want to leave connector related callbacks in drm_panel and
the rest in drm_bridge. In case of ptn3460 it does not work, ptn3460
must implement connector internally because of this limitation. I guess
it is quite typical bridge. This problem does not exists in case
of using drm_panel for ptn3460.
2. drm_bridge is attached to the encoder, this and the callback order
suggests the video data flow should be as below:
drm_crtc - drm_encoder [- drm_bridge] - drm_connector [- drm_panel]
ptn3460 implements drm_bridge and drm_connector so it suggests its
drm_bridge should be the last one, so there should be no place to add
lvds panel implemented as a drm_bridge after it, as it is done in this
patchset.
Additionally it clearly shows that there should be two categories of
drm_bridges - non-terminal and terminal.
3. drm_dev uses all-or-nothing approach, ie. it will start only when all
its components are up. It simplifies synchronization but is quite
fragile - the whole drm will be down due to error in some of its components.
For this reason I prefer drm_panel as it is not real drm component
it can be attached/detached to/from drm_connector anytime. I am not
really sure but drm_bridge does not allow for that.
So I do think we need to stick to this all-or-nothing approach for
anything that is visible to userspace
(drm_{plane,crtc,encoder,connector}). We don't currently have a way
to hotplug those so I don't see a real smooth upgrade path to add
that in a backwards compatible way that won't cause problems with old
userspace.
But, that said, we have more flexibility with things not visible to
userspace (drm_{panel,bridge}). I'm not sure how much we want to
allow things to be completely dynamic (we already have some hard
enough locking fun). But proposals/rfcs/etc welcome.
I guess I'm not completely familiar w/ ptn3460, but the fact that it
needs to implement drm_connector makes me a bit suspicious. Seems
like a symptom of missing things in drm_panel_funcs. It would be
better to always create the connector statically, and just have
_detect() - disconnected if panel==NULL.
This is something which only Sean can answer!
I guess he implemented ptn3460 as connector thinking that bridge would
be the last
entity in the video pipeline. If that's a real problem, we can still
move out the
connector part.
Regards,
Ajay
Real life example to
Re: [RFC V2 0/3] drm/bridge: panel and chaining
On 05/05/2014 09:52 PM, Ajay Kumar wrote: This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git I have just put up Rob's and Sean's idea of chaining up the bridges in code, and have implemented basic panel controls as a chained bridge. This works well with ptn3460 bridge chip on exynos5250-snow board. Still need to make use of standard list calls and figure out proper way of deleting the bridge chain. So, this is just a rough version. As I understand this patchset tries to solve two things: 1. Implement panel as drm_bridge, to ease support for hardware chains: Crtc - Encoder - Bridge - Panel 2. Add support to drm_bridge chaining, to allow software chains: drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel It is done using Russian doll schema, ops from the bridge calls the same ops from the next bridge and the next bridge ops can do the same. This schema means that all the bridges including the last one are seen from the drm core point of view as a one big drm_bridge. Additionally in this particular case, the first bridge (ptn3460) implements connector so it is hard to guess what is the location of the 2nd bridge in video stream chain, sometimes it is after the connector, sometimes before. All this is quite confusing. But if you look at the bridge from upstream video interface point of view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its video input side acts as a panel. On the output side it expects a panel, lvds panel in this case. So why not implement ptn3460 bridge as drm_panel which internally uses another drm_panel. With this approach everything fits much better. You do not need those (pre|post)_(enable|disable) calls, you do not need to implement connector in the bridge and you have a driver following linux driver model. And no single bit changed in drm core. I have implemented this way DSI/LVDS bridge, it was sent as RFC [1][2]. It was not accepted as Inki preferred drm_bridge but as I see the problems with drm_bridges I have decide to attract attention to much more cleaner solution. [1]: http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559 [2]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/27044 Regards Andrzej Ajay Kumar (3): [RFC V2 1/3] drm: implement chaining of drm bridges [RFC V2 2/3] drm/bridge: add a dummy panel driver to support lvds bridges [RFC V2 3/3] drm/bridge: ptn3460: support bridge chaining .../bindings/drm/bridge/bridge_panel.txt | 45 drivers/gpu/drm/bridge/Kconfig | 6 + drivers/gpu/drm/bridge/Makefile| 1 + drivers/gpu/drm/bridge/bridge_panel.c | 240 + drivers/gpu/drm/bridge/ptn3460.c | 21 +- drivers/gpu/drm/drm_crtc.c | 13 +- include/drm/bridge/bridge_panel.h | 37 include/drm/drm_crtc.h | 2 + 8 files changed, 360 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/drm/bridge/bridge_panel.txt create mode 100644 drivers/gpu/drm/bridge/bridge_panel.c create mode 100644 include/drm/bridge/bridge_panel.h -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC V2 0/3] drm/bridge: panel and chaining
Hi Andrzej On 2014년 05월 08일 15:41, Andrzej Hajda wrote: On 05/05/2014 09:52 PM, Ajay Kumar wrote: This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git I have just put up Rob's and Sean's idea of chaining up the bridges in code, and have implemented basic panel controls as a chained bridge. This works well with ptn3460 bridge chip on exynos5250-snow board. Still need to make use of standard list calls and figure out proper way of deleting the bridge chain. So, this is just a rough version. As I understand this patchset tries to solve two things: 1. Implement panel as drm_bridge, to ease support for hardware chains: Crtc - Encoder - Bridge - Panel 2. Add support to drm_bridge chaining, to allow software chains: drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel It is done using Russian doll schema, ops from the bridge calls the same ops from the next bridge and the next bridge ops can do the same. This schema means that all the bridges including the last one are seen from the drm core point of view as a one big drm_bridge. Additionally in this particular case, the first bridge (ptn3460) implements connector so it is hard to guess what is the location of the 2nd bridge in video stream chain, sometimes it is after the connector, sometimes before. All this is quite confusing. But if you look at the bridge from upstream video interface point of view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its video input side acts as a panel. On the output side it expects a panel, lvds panel in this case. So why not implement ptn3460 bridge as drm_panel which internally uses another drm_panel. With this approach everything fits much better. You do not need those (pre|post)_(enable|disable) calls, you do not need to implement connector in the bridge and you have a driver following linux driver model. And no single bit changed in drm core. I have implemented this way DSI/LVDS bridge, it was sent as RFC [1][2]. It was not accepted as Inki preferred drm_bridge but as I see the problems with drm_bridges I have decide to attract attention to much Yes, in above email threads, I disagreed to using drm_panel framework for bridge device, especially, to implement connector/encoder to crtc driver. However, I thought that it'd be more generic way that lvds drivers use driver-model, and the use of drm_panel infrastructure would be suitable to doing this. So my intention in turn, was that LVDS driver uses integrated drm_bridge based on drm_panel infrastructure[1], and RFC patch[2] for it. This way is same as your proposal in the point that LVDS and Panel drivers use driver-model. The only different point is that LVDS driver has some ops specific to LVDS device, not using existing ops of drm_panel commonly: we may need to consider the characteristic of LVDS device. [1]:http://www.spinics.net/lists/dri-devel/msg5.html [2]:http://www.spinics.net/lists/dri-devel/msg55658.html Thanks, Inki Dae more cleaner solution. [1]: http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559 [2]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/27044 Regards Andrzej Ajay Kumar (3): [RFC V2 1/3] drm: implement chaining of drm bridges [RFC V2 2/3] drm/bridge: add a dummy panel driver to support lvds bridges [RFC V2 3/3] drm/bridge: ptn3460: support bridge chaining .../bindings/drm/bridge/bridge_panel.txt | 45 drivers/gpu/drm/bridge/Kconfig | 6 + drivers/gpu/drm/bridge/Makefile| 1 + drivers/gpu/drm/bridge/bridge_panel.c | 240 + drivers/gpu/drm/bridge/ptn3460.c | 21 +- drivers/gpu/drm/drm_crtc.c | 13 +- include/drm/bridge/bridge_panel.h | 37 include/drm/drm_crtc.h | 2 + 8 files changed, 360 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/drm/bridge/bridge_panel.txt create mode 100644 drivers/gpu/drm/bridge/bridge_panel.c create mode 100644 include/drm/bridge/bridge_panel.h -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC V2 0/3] drm/bridge: panel and chaining
Just re-sending with text mode. Sorry for this. On 2014년 05월 08일 15:41, Andrzej Hajda wrote: On 05/05/2014 09:52 PM, Ajay Kumar wrote: This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git I have just put up Rob's and Sean's idea of chaining up the bridges in code, and have implemented basic panel controls as a chained bridge. This works well with ptn3460 bridge chip on exynos5250-snow board. Still need to make use of standard list calls and figure out proper way of deleting the bridge chain. So, this is just a rough version. As I understand this patchset tries to solve two things: 1. Implement panel as drm_bridge, to ease support for hardware chains: Crtc - Encoder - Bridge - Panel 2. Add support to drm_bridge chaining, to allow software chains: drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel It is done using Russian doll schema, ops from the bridge calls the same ops from the next bridge and the next bridge ops can do the same. This schema means that all the bridges including the last one are seen from the drm core point of view as a one big drm_bridge. Additionally in this particular case, the first bridge (ptn3460) implements connector so it is hard to guess what is the location of the 2nd bridge in video stream chain, sometimes it is after the connector, sometimes before. All this is quite confusing. But if you look at the bridge from upstream video interface point of view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its video input side acts as a panel. On the output side it expects a panel, lvds panel in this case. So why not implement ptn3460 bridge as drm_panel which internally uses another drm_panel. With this approach everything fits much better. You do not need those (pre|post)_(enable|disable) calls, you do not need to implement connector in the bridge and you have a driver following linux driver model. And no single bit changed in drm core. I have implemented this way DSI/LVDS bridge, it was sent as RFC [1][2]. It was not accepted as Inki preferred drm_bridge but as I see the Yes, in above email threads, I disagreed to using drm_panel framework for bridge device, especially, to implement connector/encoder to crtc driver. However, I thought that it'd be more generic way that lvds drivers use driver-model, and the use of drm_panel infrastructure would be suitable to doing this. So my intention in turn, was that LVDS driver uses integrated drm_bridge based on drm_panel infrastructure[1], and RFC patch[2] for it. This way is same as your proposal in the point that LVDS and Panel drivers use driver-model. The only different point is that LVDS driver has some ops specific to LVDS device, not using existing ops of drm_panel commonly: we may need to consider the characteristic of LVDS device. [1]:http://www.spinics.net/lists/dri-devel/msg5.html [2]:http://www.spinics.net/lists/dri-devel/msg55658.html Thanks, Inki Dae problems with drm_bridges I have decide to attract attention to much more cleaner solution. [1]: http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559 [2]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/27044 Regards Andrzej Ajay Kumar (3): [RFC V2 1/3] drm: implement chaining of drm bridges [RFC V2 2/3] drm/bridge: add a dummy panel driver to support lvds bridges [RFC V2 3/3] drm/bridge: ptn3460: support bridge chaining .../bindings/drm/bridge/bridge_panel.txt | 45 drivers/gpu/drm/bridge/Kconfig | 6 + drivers/gpu/drm/bridge/Makefile| 1 + drivers/gpu/drm/bridge/bridge_panel.c | 240 + drivers/gpu/drm/bridge/ptn3460.c | 21 +- drivers/gpu/drm/drm_crtc.c | 13 +- include/drm/bridge/bridge_panel.h | 37 include/drm/drm_crtc.h | 2 + 8 files changed, 360 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/drm/bridge/bridge_panel.txt create mode 100644 drivers/gpu/drm/bridge/bridge_panel.c create mode 100644 include/drm/bridge/bridge_panel.h -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC V2 0/3] drm/bridge: panel and chaining
Hi Andrej, On Thu, May 8, 2014 at 12:11 PM, Andrzej Hajda a.ha...@samsung.com wrote: On 05/05/2014 09:52 PM, Ajay Kumar wrote: This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git I have just put up Rob's and Sean's idea of chaining up the bridges in code, and have implemented basic panel controls as a chained bridge. This works well with ptn3460 bridge chip on exynos5250-snow board. Still need to make use of standard list calls and figure out proper way of deleting the bridge chain. So, this is just a rough version. As I understand this patchset tries to solve two things: 1. Implement panel as drm_bridge, to ease support for hardware chains: Crtc - Encoder - Bridge - Panel 2. Add support to drm_bridge chaining, to allow software chains: drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel It is done using Russian doll schema, ops from the bridge calls the same ops from the next bridge and the next bridge ops can do the same. This schema means that all the bridges including the last one are seen from the drm core point of view as a one big drm_bridge. Additionally in this particular case, the first bridge (ptn3460) implements connector so it is hard to guess what is the location of the 2nd bridge in video stream chain, sometimes it is after the connector, sometimes before. All this is quite confusing. But if you look at the bridge from upstream video interface point of view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its video input side acts as a panel. On the output side it expects a panel, lvds panel in this case. So why not implement ptn3460 bridge as drm_panel which internally uses another drm_panel. With this approach everything fits much better. You do not need those (pre|post)_(enable|disable) calls, you do not need to implement connector in the bridge and you have a driver following linux driver model. And no single bit changed in drm core. This discussion should have ideally happened when Sean added bridge into drm-core! And, we do need (pre|post)_(enable|disable) calls for bridges, but the drm_panel framework supports only enable/disable. I have implemented this way DSI/LVDS bridge, it was sent as RFC [1][2]. It was not accepted as Inki preferred drm_bridge but as I see the problems with drm_bridges I have decide to attract attention to much more cleaner solution. [1]: http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559 [2]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/27044 Regards Andrzej Ajay Kumar (3): [RFC V2 1/3] drm: implement chaining of drm bridges [RFC V2 2/3] drm/bridge: add a dummy panel driver to support lvds bridges [RFC V2 3/3] drm/bridge: ptn3460: support bridge chaining .../bindings/drm/bridge/bridge_panel.txt | 45 drivers/gpu/drm/bridge/Kconfig | 6 + drivers/gpu/drm/bridge/Makefile| 1 + drivers/gpu/drm/bridge/bridge_panel.c | 240 + drivers/gpu/drm/bridge/ptn3460.c | 21 +- drivers/gpu/drm/drm_crtc.c | 13 +- include/drm/bridge/bridge_panel.h | 37 include/drm/drm_crtc.h | 2 + 8 files changed, 360 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/drm/bridge/bridge_panel.txt create mode 100644 drivers/gpu/drm/bridge/bridge_panel.c create mode 100644 include/drm/bridge/bridge_panel.h -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC V2 0/3] drm/bridge: panel and chaining
+Dave +Thierry On Thu, May 8, 2014 at 1:14 PM, Inki Dae inki@samsung.com wrote: Just re-sending with text mode. Sorry for this. On 2014년 05월 08일 15:41, Andrzej Hajda wrote: On 05/05/2014 09:52 PM, Ajay Kumar wrote: This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git I have just put up Rob's and Sean's idea of chaining up the bridges in code, and have implemented basic panel controls as a chained bridge. This works well with ptn3460 bridge chip on exynos5250-snow board. Still need to make use of standard list calls and figure out proper way of deleting the bridge chain. So, this is just a rough version. As I understand this patchset tries to solve two things: 1. Implement panel as drm_bridge, to ease support for hardware chains: Crtc - Encoder - Bridge - Panel 2. Add support to drm_bridge chaining, to allow software chains: drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel It is done using Russian doll schema, ops from the bridge calls the same ops from the next bridge and the next bridge ops can do the same. This schema means that all the bridges including the last one are seen from the drm core point of view as a one big drm_bridge. Additionally in this particular case, the first bridge (ptn3460) implements connector so it is hard to guess what is the location of the 2nd bridge in video stream chain, sometimes it is after the connector, sometimes before. All this is quite confusing. But if you look at the bridge from upstream video interface point of view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its video input side acts as a panel. On the output side it expects a panel, lvds panel in this case. So why not implement ptn3460 bridge as drm_panel which internally uses another drm_panel. With this approach everything fits much better. You do not need those (pre|post)_(enable|disable) calls, you do not need to implement connector in the bridge and you have a driver following linux driver model. And no single bit changed in drm core. I have implemented this way DSI/LVDS bridge, it was sent as RFC [1][2]. It was not accepted as Inki preferred drm_bridge but as I see the Yes, in above email threads, I disagreed to using drm_panel framework for bridge device, especially, to implement connector/encoder to crtc driver. However, I thought that it'd be more generic way that lvds drivers use driver-model, and the use of drm_panel infrastructure would be suitable to doing this. So my intention in turn, was that LVDS driver uses integrated drm_bridge based on drm_panel infrastructure[1], and RFC patch[2] for it. This way is same as your proposal in the point that LVDS and Panel drivers use driver-model. The only different point is that LVDS driver has some ops specific to LVDS device, not using existing ops of drm_panel commonly: we may need to consider the characteristic of LVDS device. [1]:http://www.spinics.net/lists/dri-devel/msg5.html [2]:http://www.spinics.net/lists/dri-devel/msg55658.html Thanks, Inki Dae I am just consolidating the discussion happening here. 1) This patchset started from a discussion wherein I tried to combine drm_panel with drm_bridge. https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg28943.html 2) Sean and Rob suggested to implement a chain of bridges and then consider adding basic panel controls as a bridge. 3) Andrej's idea is to drop the existing bridge infrastructure and implement ptn3460 using drm_panel, the same way he has implemented http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559. 4) Inki's suggestion is to combine drm_bridge, drm_panel and drm_enhance into a single drm_hw_block. I am currently trying to implement (2):chaining of bridges, and I think we have not yet reached to a consensus. So adding few other people from drm community to comment. Regards, Ajay problems with drm_bridges I have decide to attract attention to much more cleaner solution. [1]: http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559 [2]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/27044 Regards Andrzej Ajay Kumar (3): [RFC V2 1/3] drm: implement chaining of drm bridges [RFC V2 2/3] drm/bridge: add a dummy panel driver to support lvds bridges [RFC V2 3/3] drm/bridge: ptn3460: support bridge chaining .../bindings/drm/bridge/bridge_panel.txt | 45 drivers/gpu/drm/bridge/Kconfig | 6 + drivers/gpu/drm/bridge/Makefile| 1 + drivers/gpu/drm/bridge/bridge_panel.c | 240 + drivers/gpu/drm/bridge/ptn3460.c | 21 +- drivers/gpu/drm/drm_crtc.c | 13 +- include/drm/bridge/bridge_panel.h | 37 include/drm/drm_crtc.h | 2 + 8 files
Re: [RFC V2 0/3] drm/bridge: panel and chaining
On 2014년 05월 08일 19:52, Ajay kumar wrote: +Dave +Thierry On Thu, May 8, 2014 at 1:14 PM, Inki Dae inki@samsung.com wrote: Just re-sending with text mode. Sorry for this. On 2014년 05월 08일 15:41, Andrzej Hajda wrote: On 05/05/2014 09:52 PM, Ajay Kumar wrote: This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git I have just put up Rob's and Sean's idea of chaining up the bridges in code, and have implemented basic panel controls as a chained bridge. This works well with ptn3460 bridge chip on exynos5250-snow board. Still need to make use of standard list calls and figure out proper way of deleting the bridge chain. So, this is just a rough version. As I understand this patchset tries to solve two things: 1. Implement panel as drm_bridge, to ease support for hardware chains: Crtc - Encoder - Bridge - Panel 2. Add support to drm_bridge chaining, to allow software chains: drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel It is done using Russian doll schema, ops from the bridge calls the same ops from the next bridge and the next bridge ops can do the same. This schema means that all the bridges including the last one are seen from the drm core point of view as a one big drm_bridge. Additionally in this particular case, the first bridge (ptn3460) implements connector so it is hard to guess what is the location of the 2nd bridge in video stream chain, sometimes it is after the connector, sometimes before. All this is quite confusing. But if you look at the bridge from upstream video interface point of view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its video input side acts as a panel. On the output side it expects a panel, lvds panel in this case. So why not implement ptn3460 bridge as drm_panel which internally uses another drm_panel. With this approach everything fits much better. You do not need those (pre|post)_(enable|disable) calls, you do not need to implement connector in the bridge and you have a driver following linux driver model. And no single bit changed in drm core. I have implemented this way DSI/LVDS bridge, it was sent as RFC [1][2]. It was not accepted as Inki preferred drm_bridge but as I see the Yes, in above email threads, I disagreed to using drm_panel framework for bridge device, especially, to implement connector/encoder to crtc driver. However, I thought that it'd be more generic way that lvds drivers use driver-model, and the use of drm_panel infrastructure would be suitable to doing this. So my intention in turn, was that LVDS driver uses integrated drm_bridge based on drm_panel infrastructure[1], and RFC patch[2] for it. This way is same as your proposal in the point that LVDS and Panel drivers use driver-model. The only different point is that LVDS driver has some ops specific to LVDS device, not using existing ops of drm_panel commonly: we may need to consider the characteristic of LVDS device. [1]:http://www.spinics.net/lists/dri-devel/msg5.html [2]:http://www.spinics.net/lists/dri-devel/msg55658.html Thanks, Inki Dae I am just consolidating the discussion happening here. 1) This patchset started from a discussion wherein I tried to combine drm_panel with drm_bridge. https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg28943.html 2) Sean and Rob suggested to implement a chain of bridges and then consider adding basic panel controls as a bridge. 3) Andrej's idea is to drop the existing bridge infrastructure and implement ptn3460 using drm_panel, the same way he has implemented http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559. 4) Inki's suggestion is to combine drm_bridge, drm_panel and drm_enhance into a single drm_hw_block. And more thing, we would need to consider image enhancer device placed between crtc and connector/encoder devices. And it'd better to rename drm_hw_block to drm_bridge, and existing drm_bridge relevant codes should be removed. Thanks, Inki Dae I am currently trying to implement (2):chaining of bridges, and I think we have not yet reached to a consensus. So adding few other people from drm community to comment. Regards, Ajay problems with drm_bridges I have decide to attract attention to much more cleaner solution. [1]: http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559 [2]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/27044 Regards Andrzej Ajay Kumar (3): [RFC V2 1/3] drm: implement chaining of drm bridges [RFC V2 2/3] drm/bridge: add a dummy panel driver to support lvds bridges [RFC V2 3/3] drm/bridge: ptn3460: support bridge chaining .../bindings/drm/bridge/bridge_panel.txt | 45 drivers/gpu/drm/bridge/Kconfig | 6 + drivers/gpu/drm/bridge/Makefile| 1 +
Re: [RFC V2 0/3] drm/bridge: panel and chaining
On 05/08/2014 12:26 PM, Ajay kumar wrote:
Hi Andrej,
On Thu, May 8, 2014 at 12:11 PM, Andrzej Hajda a.ha...@samsung.com wrote:
On 05/05/2014 09:52 PM, Ajay Kumar wrote:
This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree at:
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
I have just put up Rob's and Sean's idea of chaining up the bridges
in code, and have implemented basic panel controls as a chained bridge.
This works well with ptn3460 bridge chip on exynos5250-snow board.
Still need to make use of standard list calls and figure out proper way
of deleting the bridge chain. So, this is just a rough version.
As I understand this patchset tries to solve two things:
1. Implement panel as drm_bridge, to ease support for hardware chains:
Crtc - Encoder - Bridge - Panel
2. Add support to drm_bridge chaining, to allow software chains:
drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel
It is done using Russian doll schema, ops from the bridge calls the same
ops from the next bridge and the next bridge ops can do the same.
This schema means that all the bridges including the last one are seen
from the drm core point of view as a one big drm_bridge. Additionally in
this particular case, the first bridge (ptn3460) implements connector
so it is hard to guess what is the location of the 2nd bridge in video
stream chain, sometimes it is after the connector, sometimes before.
All this is quite confusing.
But if you look at the bridge from upstream video interface point of
view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its
video input side acts as a panel. On the output side it expects a panel,
lvds panel in this case.
So why not implement ptn3460 bridge as drm_panel which internally uses
another drm_panel. With this approach everything fits much better.
You do not need those (pre|post)_(enable|disable) calls, you do not need
to implement connector in the bridge and you have a driver following
linux driver model. And no single bit changed in drm core.
This discussion should have ideally happened when Sean added bridge
into drm-core!
Yes, I agree with this :)
And, we do need (pre|post)_(enable|disable) calls for bridges, but the
drm_panel
framework supports only enable/disable.
For true bridges pre|post can be just implemented as a part of the call,
for example:
bridge_enable()
{
/* pre-enable stuff */
panel_enable();
/* post-enable stuff */
}
And for your particular problem you have written:
The LVDS datasheet says at least 200ms delay is needed from Valid
data to BL on.
I am not sure what exactly means 'valid data' in this case, if it is
after lcd_en gpio
why not just use schedule_delayed_work. If it should be called later I
guess it should
be possible to find a proper callback to drm_panel.
Regards
Andrzej
I have implemented this way DSI/LVDS bridge, it was sent as RFC [1][2].
It was not accepted as Inki preferred drm_bridge but as I see the
problems with drm_bridges I have decide to attract attention to much
more cleaner solution.
[1]: http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559
[2]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/27044
Regards
Andrzej
Ajay Kumar (3):
[RFC V2 1/3] drm: implement chaining of drm bridges
[RFC V2 2/3] drm/bridge: add a dummy panel driver to support lvds bridges
[RFC V2 3/3] drm/bridge: ptn3460: support bridge chaining
.../bindings/drm/bridge/bridge_panel.txt | 45
drivers/gpu/drm/bridge/Kconfig | 6 +
drivers/gpu/drm/bridge/Makefile| 1 +
drivers/gpu/drm/bridge/bridge_panel.c | 240
+
drivers/gpu/drm/bridge/ptn3460.c | 21 +-
drivers/gpu/drm/drm_crtc.c | 13 +-
include/drm/bridge/bridge_panel.h | 37
include/drm/drm_crtc.h | 2 +
8 files changed, 360 insertions(+), 5 deletions(-)
create mode 100644
Documentation/devicetree/bindings/drm/bridge/bridge_panel.txt
create mode 100644 drivers/gpu/drm/bridge/bridge_panel.c
create mode 100644 include/drm/bridge/bridge_panel.h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC V2 0/3] drm/bridge: panel and chaining
On Thu, May 8, 2014 at 6:29 PM, Andrzej Hajda a.ha...@samsung.com wrote:
On 05/08/2014 12:26 PM, Ajay kumar wrote:
Hi Andrej,
On Thu, May 8, 2014 at 12:11 PM, Andrzej Hajda a.ha...@samsung.com wrote:
On 05/05/2014 09:52 PM, Ajay Kumar wrote:
This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree
at:
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
I have just put up Rob's and Sean's idea of chaining up the bridges
in code, and have implemented basic panel controls as a chained bridge.
This works well with ptn3460 bridge chip on exynos5250-snow board.
Still need to make use of standard list calls and figure out proper way
of deleting the bridge chain. So, this is just a rough version.
As I understand this patchset tries to solve two things:
1. Implement panel as drm_bridge, to ease support for hardware chains:
Crtc - Encoder - Bridge - Panel
2. Add support to drm_bridge chaining, to allow software chains:
drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel
It is done using Russian doll schema, ops from the bridge calls the same
ops from the next bridge and the next bridge ops can do the same.
This schema means that all the bridges including the last one are seen
from the drm core point of view as a one big drm_bridge. Additionally in
this particular case, the first bridge (ptn3460) implements connector
so it is hard to guess what is the location of the 2nd bridge in video
stream chain, sometimes it is after the connector, sometimes before.
All this is quite confusing.
But if you look at the bridge from upstream video interface point of
view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its
video input side acts as a panel. On the output side it expects a panel,
lvds panel in this case.
So why not implement ptn3460 bridge as drm_panel which internally uses
another drm_panel. With this approach everything fits much better.
You do not need those (pre|post)_(enable|disable) calls, you do not need
to implement connector in the bridge and you have a driver following
linux driver model. And no single bit changed in drm core.
This discussion should have ideally happened when Sean added bridge
into drm-core!
Yes, I agree with this :)
And, we do need (pre|post)_(enable|disable) calls for bridges, but the
drm_panel
framework supports only enable/disable.
For true bridges pre|post can be just implemented as a part of the call,
for example:
bridge_enable()
{
/* pre-enable stuff */
panel_enable();
/* post-enable stuff */
}
It should ideally be like this:
1) panel enable - switch on the LCD unit.
2) bridge enable - switch on the bridge.
3) encoder-commit/dpms on - valid data starts coming out of DP/MIPI
4) switch on the backlight unit and enable BL_EN.
And for your particular problem you have written:
The LVDS datasheet says at least 200ms delay is needed from Valid
data to BL on.
I am not sure what exactly means 'valid data' in this case, if it is
after lcd_en gpio
why not just use schedule_delayed_work. If it should be called later I
guess it should
be possible to find a proper callback to drm_panel.
Right. This was already discussed.
I suggested adding pre_enable/enable/disable and post_disable for drm_panel,
but Thierry was not ok with it.
Regards,
Ajay
I have implemented this way DSI/LVDS bridge, it was sent as RFC [1][2].
It was not accepted as Inki preferred drm_bridge but as I see the
problems with drm_bridges I have decide to attract attention to much
more cleaner solution.
[1]: http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559
[2]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/27044
Regards
Andrzej
Ajay Kumar (3):
[RFC V2 1/3] drm: implement chaining of drm bridges
[RFC V2 2/3] drm/bridge: add a dummy panel driver to support lvds bridges
[RFC V2 3/3] drm/bridge: ptn3460: support bridge chaining
.../bindings/drm/bridge/bridge_panel.txt | 45
drivers/gpu/drm/bridge/Kconfig | 6 +
drivers/gpu/drm/bridge/Makefile| 1 +
drivers/gpu/drm/bridge/bridge_panel.c | 240
+
drivers/gpu/drm/bridge/ptn3460.c | 21 +-
drivers/gpu/drm/drm_crtc.c | 13 +-
include/drm/bridge/bridge_panel.h | 37
include/drm/drm_crtc.h | 2 +
8 files changed, 360 insertions(+), 5 deletions(-)
create mode 100644
Documentation/devicetree/bindings/drm/bridge/bridge_panel.txt
create mode 100644 drivers/gpu/drm/bridge/bridge_panel.c
create mode 100644 include/drm/bridge/bridge_panel.h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC V2 0/3] drm/bridge: panel and chaining
On Thu, May 8, 2014 at 2:41 AM, Andrzej Hajda a.ha...@samsung.com wrote: On 05/05/2014 09:52 PM, Ajay Kumar wrote: This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git I have just put up Rob's and Sean's idea of chaining up the bridges in code, and have implemented basic panel controls as a chained bridge. This works well with ptn3460 bridge chip on exynos5250-snow board. Still need to make use of standard list calls and figure out proper way of deleting the bridge chain. So, this is just a rough version. As I understand this patchset tries to solve two things: 1. Implement panel as drm_bridge, to ease support for hardware chains: Crtc - Encoder - Bridge - Panel 2. Add support to drm_bridge chaining, to allow software chains: drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel It is done using Russian doll schema, ops from the bridge calls the same ops from the next bridge and the next bridge ops can do the same. This schema means that all the bridges including the last one are seen from the drm core point of view as a one big drm_bridge. Additionally in this particular case, the first bridge (ptn3460) implements connector so it is hard to guess what is the location of the 2nd bridge in video stream chain, sometimes it is after the connector, sometimes before. All this is quite confusing. But if you look at the bridge from upstream video interface point of view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its video input side acts as a panel. On the output side it expects a panel, lvds panel in this case. tbh, this is mostly about what we call it. Perhaps bridge isn't the best name.. I wouldn't object to changing it. But my thinking was to leave in drm_panel_funcs things that are just needed by the connector (get_modes().. and maybe some day we need detect/etc). Then leave everything else in drm_bridge_funcs. A panel could (if needed) implement both interfaces. That is basically the same as what you are proposing, but without renaming bridge to panel ;-) BR, -R So why not implement ptn3460 bridge as drm_panel which internally uses another drm_panel. With this approach everything fits much better. You do not need those (pre|post)_(enable|disable) calls, you do not need to implement connector in the bridge and you have a driver following linux driver model. And no single bit changed in drm core. I have implemented this way DSI/LVDS bridge, it was sent as RFC [1][2]. It was not accepted as Inki preferred drm_bridge but as I see the problems with drm_bridges I have decide to attract attention to much more cleaner solution. [1]: http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559 [2]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/27044 Regards Andrzej Ajay Kumar (3): [RFC V2 1/3] drm: implement chaining of drm bridges [RFC V2 2/3] drm/bridge: add a dummy panel driver to support lvds bridges [RFC V2 3/3] drm/bridge: ptn3460: support bridge chaining .../bindings/drm/bridge/bridge_panel.txt | 45 drivers/gpu/drm/bridge/Kconfig | 6 + drivers/gpu/drm/bridge/Makefile| 1 + drivers/gpu/drm/bridge/bridge_panel.c | 240 + drivers/gpu/drm/bridge/ptn3460.c | 21 +- drivers/gpu/drm/drm_crtc.c | 13 +- include/drm/bridge/bridge_panel.h | 37 include/drm/drm_crtc.h | 2 + 8 files changed, 360 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/drm/bridge/bridge_panel.txt create mode 100644 drivers/gpu/drm/bridge/bridge_panel.c create mode 100644 include/drm/bridge/bridge_panel.h -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC V2 0/3] drm/bridge: panel and chaining
On Thu, May 8, 2014 at 7:57 AM, Inki Dae inki@samsung.com wrote: On 2014년 05월 08일 19:52, Ajay kumar wrote: +Dave +Thierry On Thu, May 8, 2014 at 1:14 PM, Inki Dae inki@samsung.com wrote: Just re-sending with text mode. Sorry for this. On 2014년 05월 08일 15:41, Andrzej Hajda wrote: On 05/05/2014 09:52 PM, Ajay Kumar wrote: This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git I have just put up Rob's and Sean's idea of chaining up the bridges in code, and have implemented basic panel controls as a chained bridge. This works well with ptn3460 bridge chip on exynos5250-snow board. Still need to make use of standard list calls and figure out proper way of deleting the bridge chain. So, this is just a rough version. As I understand this patchset tries to solve two things: 1. Implement panel as drm_bridge, to ease support for hardware chains: Crtc - Encoder - Bridge - Panel 2. Add support to drm_bridge chaining, to allow software chains: drm_crtc - drm_encoder - drm_bridge - drm_bridge,panel It is done using Russian doll schema, ops from the bridge calls the same ops from the next bridge and the next bridge ops can do the same. This schema means that all the bridges including the last one are seen from the drm core point of view as a one big drm_bridge. Additionally in this particular case, the first bridge (ptn3460) implements connector so it is hard to guess what is the location of the 2nd bridge in video stream chain, sometimes it is after the connector, sometimes before. All this is quite confusing. But if you look at the bridge from upstream video interface point of view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its video input side acts as a panel. On the output side it expects a panel, lvds panel in this case. So why not implement ptn3460 bridge as drm_panel which internally uses another drm_panel. With this approach everything fits much better. You do not need those (pre|post)_(enable|disable) calls, you do not need to implement connector in the bridge and you have a driver following linux driver model. And no single bit changed in drm core. I have implemented this way DSI/LVDS bridge, it was sent as RFC [1][2]. It was not accepted as Inki preferred drm_bridge but as I see the Yes, in above email threads, I disagreed to using drm_panel framework for bridge device, especially, to implement connector/encoder to crtc driver. However, I thought that it'd be more generic way that lvds drivers use driver-model, and the use of drm_panel infrastructure would be suitable to doing this. So my intention in turn, was that LVDS driver uses integrated drm_bridge based on drm_panel infrastructure[1], and RFC patch[2] for it. This way is same as your proposal in the point that LVDS and Panel drivers use driver-model. The only different point is that LVDS driver has some ops specific to LVDS device, not using existing ops of drm_panel commonly: we may need to consider the characteristic of LVDS device. [1]:http://www.spinics.net/lists/dri-devel/msg5.html [2]:http://www.spinics.net/lists/dri-devel/msg55658.html Thanks, Inki Dae I am just consolidating the discussion happening here. 1) This patchset started from a discussion wherein I tried to combine drm_panel with drm_bridge. https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg28943.html 2) Sean and Rob suggested to implement a chain of bridges and then consider adding basic panel controls as a bridge. 3) Andrej's idea is to drop the existing bridge infrastructure and implement ptn3460 using drm_panel, the same way he has implemented http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559. 4) Inki's suggestion is to combine drm_bridge, drm_panel and drm_enhance into a single drm_hw_block. And more thing, we would need to consider image enhancer device placed between crtc and connector/encoder devices. And it'd better to rename drm_hw_block to drm_bridge, and existing drm_bridge relevant codes should be removed. I don't object to changing the name to hw_block or something else. Although to avoid introducing too much confusion in this discussion, for now I am calling it bridge/drm_bridge_funcs/etc for now ;-) BR, -R Thanks, Inki Dae I am currently trying to implement (2):chaining of bridges, and I think we have not yet reached to a consensus. So adding few other people from drm community to comment. Regards, Ajay problems with drm_bridges I have decide to attract attention to much more cleaner solution. [1]: http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559 [2]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/27044 Regards Andrzej Ajay Kumar (3): [RFC V2 1/3] drm: implement chaining of drm bridges [RFC V2 2/3] drm/bridge: add a dummy panel driver to support lvds
