Hi Andrzej,
On 07.04.2014 16:18, Andrzej Hajda wrote:
Hi Inki and Tomasz,
On 04/06/2014 05:15 AM, Inki Dae wrote:
(...)
The code creating the list of components to wait for
(exynos_drm_add_components()) doesn't seem to consider which sub-drivers are
actually enabled in kernel config.
Are you sure?
exynos_drm_add_components() will try to attach components *added to
component_lists. And these components will be added by only
corresponding sub drivers to the component_lists and
master->components.
So in this case, if users disabled HDMI support then a commponent
object for HDMI sub driver doesn't exists in the component_lists and
master->components. This means that a component object for HDMI sub
driver *cannot be attached to master object*.
As a result, component_bind_add() will ignor component_bind call for
HDMI sub driver so HDMI driver will not be bounded. The only
components added by sub drivers will be bound, component->ops->bind().
For more understanding, it seems like you need to look into below codes,
static int exynos_drm_add_components(...)
{
...
for (i == 0;; i++) {
...
node = of_parse_phandle(np, "ports", i);
...
ret = component_master_add_child(m, compare_of, node);
...
}
}
Hmm, In case HDMI driver is not present, HDMI device is not probed and
HDMI component is not added, so component_master_add_child returns
an error when it tries to add hdmi component and master is never brought
up, am I correct?
This is my thought here as well.
And below codes,
int component_master_add_child(...)
{
list_for_each_entry(c, &component_list, node) {
if (c->master)
continue;
if (compare(...)) {
component_attach_master(master, c);
...
}
}
}
And below codes,
static void component_attach_master(master, c)
{
c->master = master;
list_add_tail(&c->master_node, &master->comonents);
}
As you can see above, the only components added to component_list can
be attached to master. And the important thing is that components can
be added by sub drivers to the component_list.
And below codes that actually tries to bind each sub drivers,
int component_bind_add(...)
{
....
list_for_each_entry(c, &master->components, master_node) {
ret = component_bind(c, master, data);
...
}
...
}
The hdmi driver users disabled doesn't exist to master->components list.
How Exynos DRM cannot be initialized?
Of course, there may be my missing point, but I think I see them
correctly. Anyway I will test them tomorrow.
Thanks,
Inki Dae
register, which is completely wrong. Users should be able to select which
drivers should be compiled into their kernels.
So users are be able to select drivers they want to use, and will be
compiled correctly. So no, the only thing users can disable is each
sub driver, not core module.
3) Such approach leads to complete integration of all Exynos DRM drivers,
without possibility of loading some sub-drivers as modules. I know that
current driver design doesn't support it either, but if this series is
No, current drm driver *must also be built* as one integrated single
drm driver without super device approach.
As I wrote, I know that current design before this patch isn't modular
either, but this is not my concern here. See below.
So the super device approach
*has no any effect on existing design*, and what the super device
approch tries to do is to resolve the probe order issue to sub drivers
*without some codes specific to Exynos drm*.
My concern is that the supernode design is actually carving such broken
non-modular design in stone. Remember that we are currently heading towards
a fully multi-platform kernel where it is critical for such subsystems to be
modular, because the same zImage is going to be running on completely
different machines.
claimed to improve things, it should really do so.
4) Exactly the same can be achieved without changing the DT bindings at
all.
In fact even without adding any new single property or node to DT. We
discussed this with Andrzej and Marek today and came to a solution in
which
just by adding a little bit of code to Exynos DRM subdrivers, you could
guarantee correct registration of Exynos DRM platform and also get rid of
#ifdeffery in exynos_drm_drv.c. Andrzej will send an RFC after the
weekend.
I'm not sure but I had implemented below prototype codes for that, see
the below link,
https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?h=exynos-bridge-test&id=2860ad02d736e300034ddeabce3da92614922b4e
I guess what you said would be similler approach.
Not exactly. The approach we found does mostly the same as componentized
subsystem framework but without _any_ extra data in Device Tree. Just based
on the list of subsystem sub-drivers that is already available to the master
driver.
I am not so sure which approach is better, anyway I hope to post RFC soon,
as some material for discussion.
Great. Looking forward for it.
And I still think the use of the component framework would be the best
solution and *Linux generic way* for resolving the probe order issue
without any specific codes. So I'm not advocating the compoent
framework but I tend not to want to use specific codes.
I understand your concern. I also believe that generic frameworks should be
reused wherever possible. However the componentized subsystem framework is a
bit of overkill for this simple problem. Moreover, Device Tree is not a
trash can where any data that can be thought of can be thrown as you go, but
rather a hardware description that is supposed to be a stable ABI and needs
to be well-thought. So, if something can be done in a way that doesn't
require additional data, it's better to do it that way.
5) This series seems to break DPI display support with runtime PM
enabled.
Universal C210 just hangs on second FIMD probe, after first one fails
with
probe deferral. This needs more investigation, though.
If we are talking about the same issue, it is a problem of panel
initialization sequence on some
panels. I will post fix for it.
That's possible.
I'm not saying that it's a problem of this series itself, but it has
been introduced/triggered by this series. My intention was to point that
such issues need to be investigated before pushing the patches upstream.
Best regards,
Tomasz
--
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
