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

["Noralf Trønnes" <noralf@tronnes.org>]

Den 14.11.2017 16.56, skrev Stefan Agner:
> On 2017-11-14 16:40, Noralf Trønnes wrote:
>> 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?
> That is probably the right approach. Whatever causes the least amount of
> friction.
>
> Try to understand how to go about that:
> https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html#where-do-i-apply-my-patch
>
> Release already happened, but rc1 not yet, so I guess I should base on
> drm-misc-next-fixes now, right?

I really can't say, I've never done this before.
I suggest you ask a maintainer.
I'll respin this series with your other suggestion and drop fsl-dcu for
now until you have fixed the resume issue.

Please ping me when that is sorted and I'll send a rebased version.
No need hurry on my behalf, it doesn't block anything that can't wait a
few weeks.

Noralf.

> So I prepare a drm-fsl-dcu-next fixes-forbranch, based on
> drm-misc-next-fixes and send it to Dave.
>
> Rebasing once it is merged rebasing should rather trivial.
>
> --
> Stefan
>
>
>> 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@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