[Intel-gfx] [PATCH v3 07/10] drm/exynos: Remove custom FB helper deferred setup

[Andrzej Hajda]

On 21.03.2017 11:53, Thierry Reding wrote:
> On Tue, Mar 21, 2017 at 11:42:21AM +0100, Andrzej Hajda wrote:
>> On 21.03.2017 11:20, Daniel Vetter wrote:
>>> On Tue, Mar 21, 2017 at 10:58:43AM +0100, Andrzej Hajda wrote:
>>>> On 21.03.2017 09:13, Thierry Reding wrote:
>>>>> From: Thierry Reding <treding at nvidia.com>
>>>>>
>>>>> The FB helper core now supports deferred setup, so the driver's custom
>>>>> implementation can be removed.
>>>>>
>>>>> Signed-off-by: Thierry Reding <treding at nvidia.com>
>>>>> ---
>>>>>  drivers/gpu/drm/exynos/exynos_drm_drv.c   | 6 ++++--
>>>>>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 --
>>>>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>>>> index 09d3c4c3c858..c5a37dda8d1b 100644
>>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>>>> @@ -399,8 +399,9 @@ static int exynos_drm_bind(struct device *dev)
>>>>>  	/* init kms poll for handling hpd */
>>>>>  	drm_kms_helper_poll_init(drm);
>>>>>  
>>>>> -	/* force connectors detection */
>>>>> -	drm_helper_hpd_irq_event(drm);
>>>>> +	ret = exynos_drm_fbdev_init(dev);
>>>>> +	if (ret)
>>>>> +		goto err_cleanup_poll;
>>>> It should be rather:
>>>>     ret = exynos_drm_fbdev_init(drm);
>>>>
>>>> Even with this change it does not work, I will try to track down the
>>>> problem.
>> The solution was to remove custom equivalent of
>> drm_fb_helper_maybe_connected - exynos_drm_fbdev_is_anything_connected,
>> it is called earlier and it was a part of custom deferred fbdev.
> Cool, thanks for figuring that out. I can add that to this patch for v4.
>
>>> We might need the multi-stage fbdev setup here, i.e. init fbdev, run
>>> hpd_irq_event() (I suspect that kicks all the connectors to start
>>> probing), then initial_config for fbdev.
>>>
>>> Probably simplest way to achieve this is to keep the hpd_irq_event call,
>>> but place it _after_ the fbdev_init.
>>> -Daniel
>> I wonder if it wouldn't be sufficient to check if there is anything
>> connected, instead of checking if there is anything not-disconnected, in
>> drm_fb_helper_maybe_connected.
> My recollection is that this had been discussed when I first sent this
> out. VGA outputs are somewhat of a special case in that you can't always
> properly detect that it's connected or not. So effectively we need to
> take into account the connector_status_unknown. There's also some more
> information about this in the kerneldoc for enum drm_connector_status.

OK, I forgot about it and assumed that it is only used for initial state
of connector.
Returning to Daniel's proposition about hpd_irq_event(), it seems
unnecessary then: before calling drm_fb_helper_maybe_connected
drm_fb_helper_probe_connector_modes is called, which calls fill_modes
callback which should perform connector probing anyway, am I right?

Regards
Andrzej


>
> Thierry