On Wed, Jul 30, 2014 at 09:44:32PM +0530, Ajay kumar wrote:
On Wed, Jul 30, 2014 at 9:10 PM, Thierry Reding thierry.reding@gmail.com wrote:
On Wed, Jul 30, 2014 at 08:46:44PM +0530, Ajay kumar wrote:
On Wed, Jul 30, 2014 at 5:35 PM, Thierry Reding thierry.reding@gmail.com wrote:
On Sat, Jul 26, 2014 at 12:52:09AM +0530, Ajay Kumar wrote:
[...]
+int ptn3460_post_encoder_init(struct drm_bridge *bridge) +{
struct ptn3460_bridge *ptn_bridge = bridge->driver_private;int ret;/* bridge is attached to encoder.* safe to remove it from the bridge_lookup table.*/drm_bridge_remove_from_lookup(bridge);No, you should never do this. First, you're not adding it back to the registry when the bridge is detached, so unloading and reloading the display driver will fail. Second there should never be a need to remove it from the registry as long as the driver itself is loaded. If you're concerned about a single bridge being used multiple times, there's already code to handle that in your previous patch:
int drm_bridge_attach_encoder(...) { ... if (bridge->encoder) return -EBUSY; ... }Generally the registry should contain a list of bridges that have been registered, whether they're used or not is irrelevant.
I was just wondering if it is ok to have a node in two independent lists? bridge_lookup_table and the other mode_config.bridge_list?
Oh, it reuses the head field for the registry. I hadn't noticed before. No, you certainly can't have the same node in two lists. Honestly I don't quite understand why there was a need to expose drm_bridge as a drm_mode_object in the first place since it's never exported to userspace.
So I think that perhaps we could simply get rid of the base field and not tie in drm_bridge objects with the DRM object as we currently do. But until Sean or Rob comment on this it might be better to simply add another struct list_head field for the registry. That way both can coexist and we can independently still decide to remove the base and head fields if they're no longer needed.
Ok. What shall I name the new list_head?
"list" would be a good choice in my opinion.
.get_modes() still needs to be done from the bridge because that is the most closely connected to the display controller and therefore dictates the timing that the display controller needs to generate.
Querying the panel's .get_modes() might be useful to figure out which emulation mode to use in the bridge.
But, get_modes from panel returns me only the no_of_modes but not the actual mode structure. How do I compare the list of supported emulation modes?
You could iterate over the connector's probed_modes list which should contain all the modes that the panel reported (after .get_modes() was called).
Thierry