diff mbox series

[08/10] drm/tidss: Add dispc_is_idle()

Message ID 20231101-tidss-probe-v1-8-45149e0f9415@ideasonboard.com
State New
Headers
Series drm/tidss: Probe related fixes and cleanups |

Commit Message

Tomi Valkeinen Nov. 1, 2023, 9:17 a.m. UTC 
  Add a helper function, dispc_is_idle(), which returns whether the DSS is
idle (i.e. is any video port enabled).

For now we add a call to it in the suspend and resume callbacks, and
print a warning if in either place the DSS is not idle. In the future
this can be used to detect if the bootloader had enabled the DSS, and
the driver can act on that.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/tidss/tidss_dispc.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Laurent Pinchart Nov. 1, 2023, 2:32 p.m. UTC | #1 
Hi Tomi,

Thank you for the patch.

On Wed, Nov 01, 2023 at 11:17:45AM +0200, Tomi Valkeinen wrote:
> Add a helper function, dispc_is_idle(), which returns whether the DSS is
> idle (i.e. is any video port enabled).
> 
> For now we add a call to it in the suspend and resume callbacks, and
> print a warning if in either place the DSS is not idle.

Could you please explain here why these checks are needed/useful ? Why
would the dispc not be idle ?

> In the future
> this can be used to detect if the bootloader had enabled the DSS, and
> the driver can act on that.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/gpu/drm/tidss/tidss_dispc.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
> index 13db062892e3..a527c28c8833 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -2603,10 +2603,18 @@ void dispc_vp_setup(struct dispc_device *dispc, u32 hw_videoport,
>  	dispc_vp_set_color_mgmt(dispc, hw_videoport, state, newmodeset);
>  }
>  
> +static bool dispc_is_idle(struct dispc_device *dispc)
> +{
> +	return REG_GET(dispc, DSS_SYSSTATUS, 9, 9);
> +}
> +
>  int dispc_runtime_suspend(struct dispc_device *dispc)
>  {
>  	dev_dbg(dispc->dev, "suspend\n");
>  
> +	if (!dispc_is_idle(dispc))
> +		dev_warn(dispc->dev, "Bad HW state: DSS not idle when suspending");
> +
>  	dispc->is_enabled = false;
>  
>  	clk_disable_unprepare(dispc->fclk);
> @@ -2620,6 +2628,9 @@ int dispc_runtime_resume(struct dispc_device *dispc)
>  
>  	clk_prepare_enable(dispc->fclk);
>  
> +	if (!dispc_is_idle(dispc))
> +		dev_warn(dispc->dev, "Bad HW state: DSS not idle when resuming");
> +
>  	if (REG_GET(dispc, DSS_SYSSTATUS, 0, 0) == 0)
>  		dev_warn(dispc->dev, "DSS FUNC RESET not done!\n");
>
Tomi Valkeinen Nov. 2, 2023, 7:03 a.m. UTC | #2 
On 01/11/2023 16:32, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Wed, Nov 01, 2023 at 11:17:45AM +0200, Tomi Valkeinen wrote:
>> Add a helper function, dispc_is_idle(), which returns whether the DSS is
>> idle (i.e. is any video port enabled).
>>
>> For now we add a call to it in the suspend and resume callbacks, and
>> print a warning if in either place the DSS is not idle.
> 
> Could you please explain here why these checks are needed/useful ? Why
> would the dispc not be idle ?

I'll drop this. This was mostly a debugging aid for myself when testing 
runtime PM.

  Tomi
diff mbox series

Patch

diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
index 13db062892e3..a527c28c8833 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.c
+++ b/drivers/gpu/drm/tidss/tidss_dispc.c
@@ -2603,10 +2603,18 @@  void dispc_vp_setup(struct dispc_device *dispc, u32 hw_videoport,
 	dispc_vp_set_color_mgmt(dispc, hw_videoport, state, newmodeset);
 }
 
+static bool dispc_is_idle(struct dispc_device *dispc)
+{
+	return REG_GET(dispc, DSS_SYSSTATUS, 9, 9);
+}
+
 int dispc_runtime_suspend(struct dispc_device *dispc)
 {
 	dev_dbg(dispc->dev, "suspend\n");
 
+	if (!dispc_is_idle(dispc))
+		dev_warn(dispc->dev, "Bad HW state: DSS not idle when suspending");
+
 	dispc->is_enabled = false;
 
 	clk_disable_unprepare(dispc->fclk);
@@ -2620,6 +2628,9 @@  int dispc_runtime_resume(struct dispc_device *dispc)
 
 	clk_prepare_enable(dispc->fclk);
 
+	if (!dispc_is_idle(dispc))
+		dev_warn(dispc->dev, "Bad HW state: DSS not idle when resuming");
+
 	if (REG_GET(dispc, DSS_SYSSTATUS, 0, 0) == 0)
 		dev_warn(dispc->dev, "DSS FUNC RESET not done!\n");