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

[Noralf Trønnes]

Den 14.11.2017 16.23, skrev Stefan Agner:
> 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.

In that case shouldn't you fix that issue first with fixes tag and cc 
stable?
Then I can rebase this patch on top of that. This way we avoid merge 
conflict
when your fix is backported to 4.14 and 4.15.

Or have I missed something here?

Noralf.


> 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 at agner.ch>
>>>>> Acked-by: Stefan Agner <stefan at 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 at agner.ch>
>>>>>> Cc: Alison Wang <alison.wang at freescale.com>
>>>>>> Signed-off-by: Noralf Trønnes <noralf at 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);