Skip to content

Commit d839076

Browse files
committed
drm/xe/guc_pc: Add _locked variant for min/max freq
There are places in which the getters/setters are called one after the other causing a multiple lock()/unlock(). These are not currently a problem since they are all happening from the same thread, but there's a race possibility as calls are added outside of the early init when the max/min and stashed values need to be correlated. Add the _locked() variants to prepare for that. Reviewed-by: Rodrigo Vivi <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Lucas De Marchi <[email protected]> (cherry picked from commit 1beae9a) Signed-off-by: Lucas De Marchi <[email protected]>
1 parent afcad92 commit d839076

File tree

1 file changed

+69
-54
lines changed

1 file changed

+69
-54
lines changed

drivers/gpu/drm/xe/xe_guc_pc.c

Lines changed: 69 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
#include "xe_guc_pc.h"
77

8+
#include <linux/cleanup.h>
89
#include <linux/delay.h>
910
#include <linux/ktime.h>
1011

@@ -553,6 +554,25 @@ u32 xe_guc_pc_get_rpn_freq(struct xe_guc_pc *pc)
553554
return pc->rpn_freq;
554555
}
555556

557+
static int xe_guc_pc_get_min_freq_locked(struct xe_guc_pc *pc, u32 *freq)
558+
{
559+
int ret;
560+
561+
lockdep_assert_held(&pc->freq_lock);
562+
563+
/* Might be in the middle of a gt reset */
564+
if (!pc->freq_ready)
565+
return -EAGAIN;
566+
567+
ret = pc_action_query_task_state(pc);
568+
if (ret)
569+
return ret;
570+
571+
*freq = pc_get_min_freq(pc);
572+
573+
return 0;
574+
}
575+
556576
/**
557577
* xe_guc_pc_get_min_freq - Get the min operational frequency
558578
* @pc: The GuC PC
@@ -562,27 +582,29 @@ u32 xe_guc_pc_get_rpn_freq(struct xe_guc_pc *pc)
562582
* -EAGAIN if GuC PC not ready (likely in middle of a reset).
563583
*/
564584
int xe_guc_pc_get_min_freq(struct xe_guc_pc *pc, u32 *freq)
585+
{
586+
guard(mutex)(&pc->freq_lock);
587+
588+
return xe_guc_pc_get_min_freq_locked(pc, freq);
589+
}
590+
591+
static int xe_guc_pc_set_min_freq_locked(struct xe_guc_pc *pc, u32 freq)
565592
{
566593
int ret;
567594

568-
xe_device_assert_mem_access(pc_to_xe(pc));
595+
lockdep_assert_held(&pc->freq_lock);
569596

570-
mutex_lock(&pc->freq_lock);
571-
if (!pc->freq_ready) {
572-
/* Might be in the middle of a gt reset */
573-
ret = -EAGAIN;
574-
goto out;
575-
}
597+
/* Might be in the middle of a gt reset */
598+
if (!pc->freq_ready)
599+
return -EAGAIN;
576600

577-
ret = pc_action_query_task_state(pc);
601+
ret = pc_set_min_freq(pc, freq);
578602
if (ret)
579-
goto out;
603+
return ret;
580604

581-
*freq = pc_get_min_freq(pc);
605+
pc->user_requested_min = freq;
582606

583-
out:
584-
mutex_unlock(&pc->freq_lock);
585-
return ret;
607+
return 0;
586608
}
587609

588610
/**
@@ -595,25 +617,29 @@ int xe_guc_pc_get_min_freq(struct xe_guc_pc *pc, u32 *freq)
595617
* -EINVAL if value out of bounds.
596618
*/
597619
int xe_guc_pc_set_min_freq(struct xe_guc_pc *pc, u32 freq)
620+
{
621+
guard(mutex)(&pc->freq_lock);
622+
623+
return xe_guc_pc_set_min_freq_locked(pc, freq);
624+
}
625+
626+
static int xe_guc_pc_get_max_freq_locked(struct xe_guc_pc *pc, u32 *freq)
598627
{
599628
int ret;
600629

601-
mutex_lock(&pc->freq_lock);
602-
if (!pc->freq_ready) {
603-
/* Might be in the middle of a gt reset */
604-
ret = -EAGAIN;
605-
goto out;
606-
}
630+
lockdep_assert_held(&pc->freq_lock);
607631

608-
ret = pc_set_min_freq(pc, freq);
632+
/* Might be in the middle of a gt reset */
633+
if (!pc->freq_ready)
634+
return -EAGAIN;
635+
636+
ret = pc_action_query_task_state(pc);
609637
if (ret)
610-
goto out;
638+
return ret;
611639

612-
pc->user_requested_min = freq;
640+
*freq = pc_get_max_freq(pc);
613641

614-
out:
615-
mutex_unlock(&pc->freq_lock);
616-
return ret;
642+
return 0;
617643
}
618644

619645
/**
@@ -625,25 +651,29 @@ int xe_guc_pc_set_min_freq(struct xe_guc_pc *pc, u32 freq)
625651
* -EAGAIN if GuC PC not ready (likely in middle of a reset).
626652
*/
627653
int xe_guc_pc_get_max_freq(struct xe_guc_pc *pc, u32 *freq)
654+
{
655+
guard(mutex)(&pc->freq_lock);
656+
657+
return xe_guc_pc_get_max_freq_locked(pc, freq);
658+
}
659+
660+
static int xe_guc_pc_set_max_freq_locked(struct xe_guc_pc *pc, u32 freq)
628661
{
629662
int ret;
630663

631-
mutex_lock(&pc->freq_lock);
632-
if (!pc->freq_ready) {
633-
/* Might be in the middle of a gt reset */
634-
ret = -EAGAIN;
635-
goto out;
636-
}
664+
lockdep_assert_held(&pc->freq_lock);
637665

638-
ret = pc_action_query_task_state(pc);
666+
/* Might be in the middle of a gt reset */
667+
if (!pc->freq_ready)
668+
return -EAGAIN;
669+
670+
ret = pc_set_max_freq(pc, freq);
639671
if (ret)
640-
goto out;
672+
return ret;
641673

642-
*freq = pc_get_max_freq(pc);
674+
pc->user_requested_max = freq;
643675

644-
out:
645-
mutex_unlock(&pc->freq_lock);
646-
return ret;
676+
return 0;
647677
}
648678

649679
/**
@@ -657,24 +687,9 @@ int xe_guc_pc_get_max_freq(struct xe_guc_pc *pc, u32 *freq)
657687
*/
658688
int xe_guc_pc_set_max_freq(struct xe_guc_pc *pc, u32 freq)
659689
{
660-
int ret;
661-
662-
mutex_lock(&pc->freq_lock);
663-
if (!pc->freq_ready) {
664-
/* Might be in the middle of a gt reset */
665-
ret = -EAGAIN;
666-
goto out;
667-
}
668-
669-
ret = pc_set_max_freq(pc, freq);
670-
if (ret)
671-
goto out;
690+
guard(mutex)(&pc->freq_lock);
672691

673-
pc->user_requested_max = freq;
674-
675-
out:
676-
mutex_unlock(&pc->freq_lock);
677-
return ret;
692+
return xe_guc_pc_set_max_freq_locked(pc, freq);
678693
}
679694

680695
/**

0 commit comments

Comments
 (0)