diff mbox series

drm/rockchip: Convert drm_atomic_helper_suspend/resume()

Message ID 20180731203430.GA30136@jordon-HP-15-Notebook-PC (mailing list archive)
State New, archived
Headers show
Series drm/rockchip: Convert drm_atomic_helper_suspend/resume() | expand

Commit Message

Souptick Joarder July 31, 2018, 8:34 p.m. UTC
convert drm_atomic_helper_suspend/resume() to use
drm_mode_config_helper_suspend/resume().

With this conversion, rockchip_drm_fb_resume() and
rockchip_drm_fb_suspend() will not be used anymore.
Both of these functions can be removed.

Also, in struct rockchip_drm_private state will not be
used anymore. So this can be removed forever.

Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
Signed-off-by: Ajit Negi <ajitn.linux@gmail.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 46 ++---------------------------
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |  1 -
 2 files changed, 2 insertions(+), 45 deletions(-)

Comments

Heiko Stuebner Aug. 1, 2018, 12:33 p.m. UTC | #1
Hi Souptick,

Am Dienstag, 31. Juli 2018, 22:34:30 CEST schrieb Souptick Joarder:
> convert drm_atomic_helper_suspend/resume() to use
> drm_mode_config_helper_suspend/resume().
> 
> With this conversion, rockchip_drm_fb_resume() and
> rockchip_drm_fb_suspend() will not be used anymore.
> Both of these functions can be removed.
> 
> Also, in struct rockchip_drm_private state will not be
> used anymore. So this can be removed forever.
> 
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Signed-off-by: Ajit Negi <ajitn.linux@gmail.com>

the patch itself looks great, just a simple bookkeeping question.

What role did Ajit play in creating the patch? If I remember correctly
it is meant to be
- 1st Signed-off: Patch-Author
- 2nd Signed-off: E-Mail sender + patch possibly patch changes
(optional of course if the same)

So was this meant to be a Reviewed-by from Ajit?


Thanks
Heiko
Souptick Joarder Aug. 1, 2018, 12:43 p.m. UTC | #2
On Wed, Aug 1, 2018 at 6:03 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> Hi Souptick,
>
> Am Dienstag, 31. Juli 2018, 22:34:30 CEST schrieb Souptick Joarder:
>> convert drm_atomic_helper_suspend/resume() to use
>> drm_mode_config_helper_suspend/resume().
>>
>> With this conversion, rockchip_drm_fb_resume() and
>> rockchip_drm_fb_suspend() will not be used anymore.
>> Both of these functions can be removed.
>>
>> Also, in struct rockchip_drm_private state will not be
>> used anymore. So this can be removed forever.
>>
>> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
>> Signed-off-by: Ajit Negi <ajitn.linux@gmail.com>
>
> the patch itself looks great, just a simple bookkeeping question.
>
> What role did Ajit play in creating the patch? If I remember correctly
> it is meant to be
> - 1st Signed-off: Patch-Author
> - 2nd Signed-off: E-Mail sender + patch possibly patch changes
> (optional of course if the same)
>
> So was this meant to be a Reviewed-by from Ajit?

We both are working together for these patches to
convert drm_atomic_helper_suspend/resume().
That's the reason to add his name in 2nd Signed-off
in all similar patches.

Is it a incorrect way to put 2nd Signed-off here ?
Heiko Stuebner Aug. 1, 2018, 1:10 p.m. UTC | #3
Am Mittwoch, 1. August 2018, 14:43:47 CEST schrieb Souptick Joarder:
> On Wed, Aug 1, 2018 at 6:03 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> > Hi Souptick,
> >
> > Am Dienstag, 31. Juli 2018, 22:34:30 CEST schrieb Souptick Joarder:
> >> convert drm_atomic_helper_suspend/resume() to use
> >> drm_mode_config_helper_suspend/resume().
> >>
> >> With this conversion, rockchip_drm_fb_resume() and
> >> rockchip_drm_fb_suspend() will not be used anymore.
> >> Both of these functions can be removed.
> >>
> >> Also, in struct rockchip_drm_private state will not be
> >> used anymore. So this can be removed forever.
> >>
> >> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> >> Signed-off-by: Ajit Negi <ajitn.linux@gmail.com>
> >
> > the patch itself looks great, just a simple bookkeeping question.
> >
> > What role did Ajit play in creating the patch? If I remember correctly
> > it is meant to be
> > - 1st Signed-off: Patch-Author
> > - 2nd Signed-off: E-Mail sender + patch possibly patch changes
> > (optional of course if the same)
> >
> > So was this meant to be a Reviewed-by from Ajit?
> 
> We both are working together for these patches to
> convert drm_atomic_helper_suspend/resume().
> That's the reason to add his name in 2nd Signed-off
> in all similar patches.
> 
> Is it a incorrect way to put 2nd Signed-off here ?

Thanks for the clarification and the interesting question :-)

I've just read through Documentation/process/submitting-patches.rst
and it seems there is an "official" way to show that relationship, via a
tag named "Co-Developed-by:" described under number 12.

So I guess we could just adapt the patch accordingly, if that is ok with
both of you (i.e. I can change this when applying, so no need to resend).


Heiko
Souptick Joarder Aug. 1, 2018, 1:14 p.m. UTC | #4
On Wed, Aug 1, 2018 at 6:40 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> Am Mittwoch, 1. August 2018, 14:43:47 CEST schrieb Souptick Joarder:
>> On Wed, Aug 1, 2018 at 6:03 PM, Heiko Stuebner <heiko@sntech.de> wrote:
>> > Hi Souptick,
>> >
>> > Am Dienstag, 31. Juli 2018, 22:34:30 CEST schrieb Souptick Joarder:
>> >> convert drm_atomic_helper_suspend/resume() to use
>> >> drm_mode_config_helper_suspend/resume().
>> >>
>> >> With this conversion, rockchip_drm_fb_resume() and
>> >> rockchip_drm_fb_suspend() will not be used anymore.
>> >> Both of these functions can be removed.
>> >>
>> >> Also, in struct rockchip_drm_private state will not be
>> >> used anymore. So this can be removed forever.
>> >>
>> >> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
>> >> Signed-off-by: Ajit Negi <ajitn.linux@gmail.com>
>> >
>> > the patch itself looks great, just a simple bookkeeping question.
>> >
>> > What role did Ajit play in creating the patch? If I remember correctly
>> > it is meant to be
>> > - 1st Signed-off: Patch-Author
>> > - 2nd Signed-off: E-Mail sender + patch possibly patch changes
>> > (optional of course if the same)
>> >
>> > So was this meant to be a Reviewed-by from Ajit?
>>
>> We both are working together for these patches to
>> convert drm_atomic_helper_suspend/resume().
>> That's the reason to add his name in 2nd Signed-off
>> in all similar patches.
>>
>> Is it a incorrect way to put 2nd Signed-off here ?
>
> Thanks for the clarification and the interesting question :-)
>
> I've just read through Documentation/process/submitting-patches.rst
> and it seems there is an "official" way to show that relationship, via a
> tag named "Co-Developed-by:" described under number 12.
>
> So I guess we could just adapt the patch accordingly, if that is ok with
> both of you (i.e. I can change this when applying, so no need to resend).

We are ok with it :-)

>
>
> Heiko
>
>
Heiko Stuebner Aug. 1, 2018, 1:32 p.m. UTC | #5
Am Mittwoch, 1. August 2018, 15:14:24 CEST schrieb Souptick Joarder:
> On Wed, Aug 1, 2018 at 6:40 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> > Am Mittwoch, 1. August 2018, 14:43:47 CEST schrieb Souptick Joarder:
> >> On Wed, Aug 1, 2018 at 6:03 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> >> > Hi Souptick,
> >> >
> >> > Am Dienstag, 31. Juli 2018, 22:34:30 CEST schrieb Souptick Joarder:
> >> >> convert drm_atomic_helper_suspend/resume() to use
> >> >> drm_mode_config_helper_suspend/resume().
> >> >>
> >> >> With this conversion, rockchip_drm_fb_resume() and
> >> >> rockchip_drm_fb_suspend() will not be used anymore.
> >> >> Both of these functions can be removed.
> >> >>
> >> >> Also, in struct rockchip_drm_private state will not be
> >> >> used anymore. So this can be removed forever.
> >> >>
> >> >> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> >> >> Signed-off-by: Ajit Negi <ajitn.linux@gmail.com>
> >> >
> >> > the patch itself looks great, just a simple bookkeeping question.
> >> >
> >> > What role did Ajit play in creating the patch? If I remember correctly
> >> > it is meant to be
> >> > - 1st Signed-off: Patch-Author
> >> > - 2nd Signed-off: E-Mail sender + patch possibly patch changes
> >> > (optional of course if the same)
> >> >
> >> > So was this meant to be a Reviewed-by from Ajit?
> >>
> >> We both are working together for these patches to
> >> convert drm_atomic_helper_suspend/resume().
> >> That's the reason to add his name in 2nd Signed-off
> >> in all similar patches.
> >>
> >> Is it a incorrect way to put 2nd Signed-off here ?
> >
> > Thanks for the clarification and the interesting question :-)
> >
> > I've just read through Documentation/process/submitting-patches.rst
> > and it seems there is an "official" way to show that relationship, via a
> > tag named "Co-Developed-by:" described under number 12.
> >
> > So I guess we could just adapt the patch accordingly, if that is ok with
> > both of you (i.e. I can change this when applying, so no need to resend).
> 
> We are ok with it :-)

applied to drm-misc-next with the second signed-off changed to
co-developed-by.

Thanks for that nice cleanup.
Heiko
diff mbox series

Patch

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index f814d37..dd12390 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -243,60 +243,18 @@  static void rockchip_drm_unbind(struct device *dev)
 };
 
 #ifdef CONFIG_PM_SLEEP
-static void rockchip_drm_fb_suspend(struct drm_device *drm)
-{
-	struct rockchip_drm_private *priv = drm->dev_private;
-
-	console_lock();
-	drm_fb_helper_set_suspend(&priv->fbdev_helper, 1);
-	console_unlock();
-}
-
-static void rockchip_drm_fb_resume(struct drm_device *drm)
-{
-	struct rockchip_drm_private *priv = drm->dev_private;
-
-	console_lock();
-	drm_fb_helper_set_suspend(&priv->fbdev_helper, 0);
-	console_unlock();
-}
-
 static int rockchip_drm_sys_suspend(struct device *dev)
 {
 	struct drm_device *drm = dev_get_drvdata(dev);
-	struct rockchip_drm_private *priv;
-
-	if (!drm)
-		return 0;
-
-	drm_kms_helper_poll_disable(drm);
-	rockchip_drm_fb_suspend(drm);
 
-	priv = drm->dev_private;
-	priv->state = drm_atomic_helper_suspend(drm);
-	if (IS_ERR(priv->state)) {
-		rockchip_drm_fb_resume(drm);
-		drm_kms_helper_poll_enable(drm);
-		return PTR_ERR(priv->state);
-	}
-
-	return 0;
+	return drm_mode_config_helper_suspend(drm);
 }
 
 static int rockchip_drm_sys_resume(struct device *dev)
 {
 	struct drm_device *drm = dev_get_drvdata(dev);
-	struct rockchip_drm_private *priv;
 
-	if (!drm)
-		return 0;
-
-	priv = drm->dev_private;
-	drm_atomic_helper_resume(drm, priv->state);
-	rockchip_drm_fb_resume(drm);
-	drm_kms_helper_poll_enable(drm);
-
-	return 0;
+	return drm_mode_config_helper_resume(drm);
 }
 #endif
 
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index 3a6ebfc..d67ad0a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -51,7 +51,6 @@  struct rockchip_crtc_state {
 struct rockchip_drm_private {
 	struct drm_fb_helper fbdev_helper;
 	struct drm_gem_object *fbdev_bo;
-	struct drm_atomic_state *state;
 	struct iommu_domain *domain;
 	struct mutex mm_lock;
 	struct drm_mm mm;