[PATCH 3/3] drm/v3d: Add job timeout module param

[Chema Casanova]

On 3/9/20 18:48, Yukimasa Sugizaki wrote:
> From: Yukimasa Sugizaki <ysugi at idein.jp>
>
> The default timeout is 500 ms which is too short for some workloads
> including Piglit.  Adding this parameter will help users to run heavier
> tasks.
>
> Signed-off-by: Yukimasa Sugizaki <ysugi at idein.jp>
> ---
>   drivers/gpu/drm/v3d/v3d_sched.c | 24 +++++++++++++-----------
>   1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
> index feef0c749e68..983efb018560 100644
> --- a/drivers/gpu/drm/v3d/v3d_sched.c
> +++ b/drivers/gpu/drm/v3d/v3d_sched.c
> @@ -19,11 +19,17 @@
>    */
>
>   #include <linux/kthread.h>
> +#include <linux/moduleparam.h>
>
>   #include "v3d_drv.h"
>   #include "v3d_regs.h"
>   #include "v3d_trace.h"
>
> +static uint timeout = 500;
> +module_param(timeout, uint, 0444);
> +MODULE_PARM_DESC(timeout,
> +	"Timeout for a job in ms (0 means infinity and default is 500 ms)");
> +

I think that  parameter definition could be included at v3d_drv.c as 
other drivers do. Then we would have all future parameters together. In 
that case we would need also to include the extern definition at 
v3d_drv.h for the driver variable.

The param name could be more descriptive like "sched_timeout_ms" in the 
lima driver.

I wonder if it would make sense to have different timeouts parameters 
for different type of jobs we have. At least this series come from the 
need additional time to complete compute jobs. This is what amdgpu does, 
but we probably don't need something such complex.

I think it would also make sense to increase our default value for the 
compute jobs.

What do you think?

In any case I think that having this general scheduling timeout 
parameter as other drivers do is a good idea.

Regards,

Chema Casanova

>   static struct v3d_job *
>   to_v3d_job(struct drm_sched_job *sched_job)
>   {
> @@ -432,13 +438,13 @@ v3d_sched_init(struct v3d_dev *v3d)
>   {
>   	int hw_jobs_limit = 1;
>   	int job_hang_limit = 0;
> -	int hang_limit_ms = 500;
> +	long timeout_jiffies = timeout ?
> +			msecs_to_jiffies(timeout) : MAX_SCHEDULE_TIMEOUT;
>   	int ret;
>
>   	ret = drm_sched_init(&v3d->queue[V3D_BIN].sched,
>   			     &v3d_bin_sched_ops,
> -			     hw_jobs_limit, job_hang_limit,
> -			     msecs_to_jiffies(hang_limit_ms),
> +			     hw_jobs_limit, job_hang_limit, timeout_jiffies,
>   			     "v3d_bin");
>   	if (ret) {
>   		dev_err(v3d->drm.dev, "Failed to create bin scheduler: %d.", ret);
> @@ -447,8 +453,7 @@ v3d_sched_init(struct v3d_dev *v3d)
>
>   	ret = drm_sched_init(&v3d->queue[V3D_RENDER].sched,
>   			     &v3d_render_sched_ops,
> -			     hw_jobs_limit, job_hang_limit,
> -			     msecs_to_jiffies(hang_limit_ms),
> +			     hw_jobs_limit, job_hang_limit, timeout_jiffies,
>   			     "v3d_render");
>   	if (ret) {
>   		dev_err(v3d->drm.dev, "Failed to create render scheduler: %d.",
> @@ -459,8 +464,7 @@ v3d_sched_init(struct v3d_dev *v3d)
>
>   	ret = drm_sched_init(&v3d->queue[V3D_TFU].sched,
>   			     &v3d_tfu_sched_ops,
> -			     hw_jobs_limit, job_hang_limit,
> -			     msecs_to_jiffies(hang_limit_ms),
> +			     hw_jobs_limit, job_hang_limit, timeout_jiffies,
>   			     "v3d_tfu");
>   	if (ret) {
>   		dev_err(v3d->drm.dev, "Failed to create TFU scheduler: %d.",
> @@ -472,8 +476,7 @@ v3d_sched_init(struct v3d_dev *v3d)
>   	if (v3d_has_csd(v3d)) {
>   		ret = drm_sched_init(&v3d->queue[V3D_CSD].sched,
>   				     &v3d_csd_sched_ops,
> -				     hw_jobs_limit, job_hang_limit,
> -				     msecs_to_jiffies(hang_limit_ms),
> +				     hw_jobs_limit, job_hang_limit, timeout_jiffies,
>   				     "v3d_csd");
>   		if (ret) {
>   			dev_err(v3d->drm.dev, "Failed to create CSD scheduler: %d.",
> @@ -484,8 +487,7 @@ v3d_sched_init(struct v3d_dev *v3d)
>
>   		ret = drm_sched_init(&v3d->queue[V3D_CACHE_CLEAN].sched,
>   				     &v3d_cache_clean_sched_ops,
> -				     hw_jobs_limit, job_hang_limit,
> -				     msecs_to_jiffies(hang_limit_ms),
> +				     hw_jobs_limit, job_hang_limit, timeout_jiffies,
>   				     "v3d_cache_clean");
>   		if (ret) {
>   			dev_err(v3d->drm.dev, "Failed to create CACHE_CLEAN scheduler: %d.",
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>