Re: [PATCH v2 4/6] drm/fsl-dcu: Use drm_mode_config_helper_suspend/resume()

[Stefan Agner <stefan@agner.ch>]

On 2017-11-10 19:06, Noralf Trønnes wrote:
> Den 10.11.2017 17.39, skrev Stefan Agner:
>> On 2017-11-09 17:49, Noralf Trønnes wrote:
>>> Den 09.11.2017 15.34, skrev Stefan Agner:
>>>> On 2017-11-06 20:18, Noralf Trønnes wrote:
>>>>> Replace driver's code with the generic helpers that do the same thing.
>>>> Tested using:
>>>> echo devices > /sys/power/pm_test
>>>> echo freeze > /sys/power/state
>>>>
>>>>
>>>> Note, currently I do get, but even without this patches, so this is
>>>> something else:
>>>>
>>>> [  930.992433] ------------[ cut here ]------------
>>>> [  930.992494] WARNING: CPU: 0 PID: 361 at
>>>> drivers/gpu/drm/drm_atomic_helper.c:1249
>>>> drm_atomic_helper_wait_for_vblanks.part.1+0x284/0x288
>>>> [  930.992502] [CRTC:28:crtc-0] vblank wait timed out
>> I resolved that issue and another related to suspend/resume for the DCU
>> driver:
>> https://patchwork.freedesktop.org/series/33616/
>>
>> Suspend/resume is not supported for the platform (Vybrid) I usually test
>> on, so that is why I did not catch this earlier.
>>
>> This two patches are now on-top of your changes. How can we make sure
>> this goes through smoothly? For which merge window are you targeting
>> your changes?
> 
> drm-misc is always open so I'm planning to apply it this weekend or
> maybe monday.
> This means it will go into 4.16. Maybe you need to fix it before that?

The issues were already present in 4.14, so anyway to late for that. The
suspend/resume platform support for the SoC using this IP is missing in
mainline anyway, so in practice this patches are not that critical. I
probably will backport it for v4.14 since its LTS once it hits mainline.

So from my point of view, 4.16 is fine.

Should I create a separate pull request for those or can you pick them
up directly?

--
Stefan


> 
> Could you help me out by acking the tinydrm patch in this series?
> 
> Noralf.
> 
>> --
>> Stefan
>>
>>>>
>>>> Tested-by: Stefan Agner <stefan@agner.ch>
>>>> Acked-by: Stefan Agner <stefan@agner.ch>
>>>>
>>>> Will you take the patch through drm-misc?
>>> Yes if that's fine with you, thanks for testing.
>>>
>>> Noralf.
>>>
>>>> --
>>>> Stefan
>>>>
>>>>
>>>>
>>>>> Cc: Stefan Agner <stefan@agner.ch>
>>>>> Cc: Alison Wang <alison.wang@freescale.com>
>>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>>> ---
>>>>>    drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 24 ++++++------------------
>>>>>    drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h |  1 -
>>>>>    2 files changed, 6 insertions(+), 19 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>>>>> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>>>>> index 58e9e0601a61..1a9ee657bbac 100644
>>>>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>>>>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>>>>> @@ -27,6 +27,7 @@
>>>>>    #include <drm/drm_crtc_helper.h>
>>>>>    #include <drm/drm_fb_cma_helper.h>
>>>>>    #include <drm/drm_gem_cma_helper.h>
>>>>> +#include <drm/drm_modeset_helper.h>
>>>>>      #include "fsl_dcu_drm_crtc.h"
>>>>>    #include "fsl_dcu_drm_drv.h"
>>>>> @@ -188,26 +189,17 @@ static struct drm_driver fsl_dcu_drm_driver = {
>>>>>    static int fsl_dcu_drm_pm_suspend(struct device *dev)
>>>>>    {
>>>>>    	struct fsl_dcu_drm_device *fsl_dev = dev_get_drvdata(dev);
>>>>> +	int ret;
>>>>>      	if (!fsl_dev)
>>>>>    		return 0;
>>>>>      	disable_irq(fsl_dev->irq);
>>>>> -	drm_kms_helper_poll_disable(fsl_dev->drm);
>>>>>    -	console_lock();
>>>>> -	drm_fbdev_cma_set_suspend(fsl_dev->fbdev, 1);
>>>>> -	console_unlock();
>>>>> -
>>>>> -	fsl_dev->state = drm_atomic_helper_suspend(fsl_dev->drm);
>>>>> -	if (IS_ERR(fsl_dev->state)) {
>>>>> -		console_lock();
>>>>> -		drm_fbdev_cma_set_suspend(fsl_dev->fbdev, 0);
>>>>> -		console_unlock();
>>>>> -
>>>>> -		drm_kms_helper_poll_enable(fsl_dev->drm);
>>>>> +	ret = drm_mode_config_helper_suspend(fsl_dev->drm);
>>>>> +	if (ret) {
>>>>>    		enable_irq(fsl_dev->irq);
>>>>> -		return PTR_ERR(fsl_dev->state);
>>>>> +		return ret;
>>>>>    	}
>>>>>      	clk_disable_unprepare(fsl_dev->pix_clk);
>>>>> @@ -233,13 +225,9 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
>>>>>    	if (fsl_dev->tcon)
>>>>>    		fsl_tcon_bypass_enable(fsl_dev->tcon);
>>>>>    	fsl_dcu_drm_init_planes(fsl_dev->drm);
>>>>> -	drm_atomic_helper_resume(fsl_dev->drm, fsl_dev->state);
>>>>>    -	console_lock();
>>>>> -	drm_fbdev_cma_set_suspend(fsl_dev->fbdev, 0);
>>>>> -	console_unlock();
>>>>> +	drm_mode_config_helper_resume(fsl_dev->drm);
>>>>>    -	drm_kms_helper_poll_enable(fsl_dev->drm);
>>>>>    	enable_irq(fsl_dev->irq);
>>>>>      	return 0;
>>>>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>>>>> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>>>>> index da9bfd432ca6..93bfb98012d4 100644
>>>>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>>>>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>>>>> @@ -196,7 +196,6 @@ struct fsl_dcu_drm_device {
>>>>>    	struct drm_encoder encoder;
>>>>>    	struct fsl_dcu_drm_connector connector;
>>>>>    	const struct fsl_dcu_soc_data *soc;
>>>>> -	struct drm_atomic_state *state;
>>>>>    };
>>>>>      int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device *fsl_dev);
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel