[PATCH] drm/atomic-helper: Do not call ->mode_fixup for CRTC which will be disabled

[PATCH] drm/atomic-helper: Do not call ->mode_fixup for CRTC which will be disabled

[Liu Ying]

When a CRTC is going to be disabled, it's state may contain a display mode
with zeroed content.  This could be reproduced by HDMI cable hotplug out
operation with legacy fbdev support in dual display cases.  It would confuse
driver's CRTC callback ->mode_fixup and make the total state be rejected.
So, let's don't call the callback for the CRTC.

Signed-off-by: Liu Ying <gnuiyl@gmail.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index ddfa0d1..939df90 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -414,6 +414,9 @@ mode_fixup(struct drm_atomic_state *state)
 	for_each_crtc_in_state(state, crtc, crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
+		if (!crtc_state->enable)
+			continue;
+
 		if (!crtc_state->mode_changed &&
 		    !crtc_state->connectors_changed)
 			continue;
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/atomic-helper: Do not call ->mode_fixup for CRTC which will be disabled

[Daniel Vetter]

On Fri, May 27, 2016 at 05:35:54PM +0800, Liu Ying wrote:
> When a CRTC is going to be disabled, it's state may contain a display mode
> with zeroed content.  This could be reproduced by HDMI cable hotplug out
> operation with legacy fbdev support in dual display cases.  It would confuse
> driver's CRTC callback ->mode_fixup and make the total state be rejected.
> So, let's don't call the callback for the CRTC.
> 
> Signed-off-by: Liu Ying <gnuiyl@gmail.com>

Indeed this matches behaviour for encoder/bridge mode_fixup functions.
Patch merged to drm-misc.

A doc patch for the kernel doc of all the mode_fixup (and the encoder's
atomic_check) that this is only called when enabling a mode would be
awesome.
-Daniel

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ddfa0d1..939df90 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -414,6 +414,9 @@ mode_fixup(struct drm_atomic_state *state)
>  	for_each_crtc_in_state(state, crtc, crtc_state, i) {
>  		const struct drm_crtc_helper_funcs *funcs;
>  
> +		if (!crtc_state->enable)
> +			continue;
> +
>  		if (!crtc_state->mode_changed &&
>  		    !crtc_state->connectors_changed)
>  			continue;
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/atomic-helper: Do not call ->mode_fixup for CRTC which will be disabled

[Ying Liu]

On Mon, May 30, 2016 at 4:46 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, May 27, 2016 at 05:35:54PM +0800, Liu Ying wrote:
>> When a CRTC is going to be disabled, it's state may contain a display mode
>> with zeroed content.  This could be reproduced by HDMI cable hotplug out
>> operation with legacy fbdev support in dual display cases.  It would confuse
>> driver's CRTC callback ->mode_fixup and make the total state be rejected.
>> So, let's don't call the callback for the CRTC.
>>
>> Signed-off-by: Liu Ying <gnuiyl@gmail.com>
>
> Indeed this matches behaviour for encoder/bridge mode_fixup functions.
> Patch merged to drm-misc.
>
> A doc patch for the kernel doc of all the mode_fixup (and the encoder's
> atomic_check) that this is only called when enabling a mode would be
> awesome.

I'll write one.

Regards,
Liu Ying

> -Daniel
>
>> ---
>>  drivers/gpu/drm/drm_atomic_helper.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index ddfa0d1..939df90 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -414,6 +414,9 @@ mode_fixup(struct drm_atomic_state *state)
>>       for_each_crtc_in_state(state, crtc, crtc_state, i) {
>>               const struct drm_crtc_helper_funcs *funcs;
>>
>> +             if (!crtc_state->enable)
>> +                     continue;
>> +
>>               if (!crtc_state->mode_changed &&
>>                   !crtc_state->connectors_changed)
>>                       continue;
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel