[PATCH] drm/exynos: only run atomic_check() if crtc is active

[Andrzej Hajda]

Hi Daniel,

On 11/17/2015 11:06 AM, Daniel Vetter wrote:
> On Thu, Nov 12, 2015 at 02:49:29PM +0100, Thierry Reding wrote:
>> On Thu, Nov 12, 2015 at 11:11:20AM -0200, Gustavo Padovan wrote:
>>> From: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
>>>
>>> Fixes an regression added by 3ae2436 (drm/exynos/mixer: replace
>>> direct cross-driver call with drm mode). The whole atomic update
>>> was failing if the hdmi display is not present/active. Add a test
>>> to only run atomic_check() if the CRTC is active.
>>>
>>> Signed-off-by: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_drm_crtc.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>>> index b3ba27f..1d3ca0a 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>>> @@ -55,6 +55,9 @@ static int exynos_crtc_atomic_check(struct drm_crtc *crtc,
>>>  {
>>>  	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
>>>  
>>> +	if (!state->active)
>>> +		return 0;
>>> +
>>>  	if (exynos_crtc->ops->atomic_check)
>>>  		return exynos_crtc->ops->atomic_check(exynos_crtc, state);
>>>  
>>
>> This looks like something that the core should be doing.
> 
> Nah, this is a bug in exynos atomic_check code. Drivers _must_ check state
> irrespective of state->active - if they forgoe any checking when active =
> false then legacy DPMS On might spuriuosly fail, which will upset
> userspace. There shouldn't be any exceptions to this rule.
> 
> Cheers, Daniel
> 

What about the situation when we have two display pipelines
with separate crtcs:
- one for panel, fimd->dsi->panel,
- one for hdmi, mixer->hdmi->TV.

Since TV is not connected mixer state have mode initially filled
with zeros and active field set to 0. How should we handle situation
if userspace tries to enable dpms on HDMI connector?
How should we handle situation userspace tries to start panel pipeline?
In this case atomic_check for mixer is called also, but since
it will not be used and its state will not be changed it
should not return error, am I right?

Regards
Andrzej