linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: tomasz.figa@gmail.com (Tomasz Figa)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 1/7] drm/exynos: add super device support
Date: Mon, 07 Apr 2014 17:40:42 +0200	[thread overview]
Message-ID: <5342C6FA.7060201@gmail.com> (raw)
In-Reply-To: <CAAQKjZNf=Lr=9DwFM4KCia8jeUpsVu-6h9RyfFpqK+8kkDsbgA@mail.gmail.com>

On 07.04.2014 17:16, Inki Dae wrote:
> Hi Andrzej,
>
> 2014-04-07 23:18 GMT+09:00 Andrzej Hajda <a.hajda@samsung.com>:
>> 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?
>>
>
> Ok, after that,
>
> ret = component_master_add(,..)
> if (ret < 0)
>           DRM_DEBUG_KMS("re-tried by last sub driver probed later.\n");
>
> As you can see above, if component_master_add() is failed, return 0,
> *not error*. And then component_add() called by other kms drivers,
> late probing of hdmi or fimd probing - if there is any kms driver
> enabled - will try to bring up master again by calling
> try_to_bring_up_master() from beginning.
>
> And if component_list is empty then it means that there is no any kms
> driver enabled.
>
> Do you still think in that case, exynos drm initialization will be failed?

There will be no HDMI driver to call component_add(), because HDMI sub 
driver will be disabled in kernel config and any previous 
component_master_add_child() for the phandle pointing to HDMI node will 
wail, because such component will never be registered.

The complete failure path is as follows:

exynos_drm_platform_probe()
	component_master_add()
		try_to_bring_up_master()
			exynos_drm_add_components()
				// phandle to HDMI node
				component_master_add_child()
				= -ENXIO
			= -ENXIO
		= 0 // but no call to master->ops->bind(master->dev);
	= 0
= 0 // but Exynos DRM platform is not registered yet

Now if any other late-probed sub-driver comes, the sequence will be as 
follows (let's take FIMD as an example):

fimd_probe()
	component_add()
		try_to_bring_up_masters()
			try_to_bring_up_master()
				exynos_drm_add_components()
					// phandle to HDMI node
					component_master_add_child()
					= -ENXIO
				= -ENXIO
			= 0 // but no call to 	
			    // master->ops->bind(master->dev);
		= 0
	= 0
= 0 // but Exynos DRM platform still not registered

And the same for any late-probed driver, unless HDMI drivers loads, but 
there is no HDMI driver, because it is disabled in kernel config and not 
compiled in.

Best regards,
Tomasz

  reply	other threads:[~2014-04-07 15:40 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1396355882-17010-1-git-send-email-inki.dae@samsung.com>
     [not found] ` <1396355882-17010-2-git-send-email-inki.dae@samsung.com>
     [not found]   ` <533EB9C6.80302@samsung.com>
     [not found]     ` <CAAQKjZPbzb+qvJA5y+Kco9BRv4j3iVatM_0bd+ugyuVZUbfFFg@mail.gmail.com>
2014-04-05 17:32       ` [PATCH v2 1/7] drm/exynos: add super device support Tomasz Figa
2014-04-05 18:24         ` Russell King - ARM Linux
2014-04-05 18:31           ` Tomasz Figa
2014-04-05 18:52             ` Russell King - ARM Linux
2014-04-05 19:01               ` Tomasz Figa
2014-04-06  4:21             ` Inki Dae
2014-04-06  8:47               ` Russell King - ARM Linux
2014-04-06  9:38                 ` Inki Dae
2014-04-06  3:15         ` Inki Dae
2014-04-07 14:18           ` Andrzej Hajda
2014-04-07 14:24             ` Tomasz Figa
2014-04-07 15:16             ` Inki Dae
2014-04-07 15:40               ` Tomasz Figa [this message]
2014-04-08  8:37                 ` Inki Dae

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5342C6FA.7060201@gmail.com \
    --to=tomasz.figa@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).