On Wed, Sep 14, 2022 at 09:04:10AM -0700, Dixit, Ashutosh wrote:
On Tue, 23 Aug 2022 13:41:42 -0700, Umesh Nerlige Ramappa wrote:
Hi Umesh,
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 6fc4f0d8fc5a..bbf1c574f393 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -385,6 +385,21 @@ static struct ctl_table_header *sysctl_header;
static enum hrtimer_restart oa_poll_check_timer_cb(struct hrtimer *hrtimer);
+static u32 _oa_taken(struct i915_perf_stream * stream, u32 tail, u32 head)
nit: no space between * and stream.
+{
+ u32 size = stream->oa_buffer.vma->size;
+
+ return tail >= head ? tail - head : size - (head - tail);
+}
If we are doing this we should probably eliminate references to OA_TAKEN
which serves an identical purpose (I think there is one remaining
reference) and also delete OA_TAKEN #define.
+
+static u32 _rewind_tail(struct i915_perf_stream * stream, u32 relative_hw_tail,
+ u32 rewind_delta)
+{
+ return rewind_delta > relative_hw_tail ?
+ stream->oa_buffer.vma->size - (rewind_delta - relative_hw_tail) :
+ relative_hw_tail - rewind_delta;
+}
Also are we really saying here that we are supporting non-power-of-2 OA
buffer sizes? Because if we stayed with power-of-2 sizes the expression
above are nice and elegant and actually closer to the previous code being
changed in this patch. For example:
#include <linux/circ_buf.h>
static u32 _oa_taken(struct i915_perf_stream *stream, u32 tail, u32 head)
{
return CIRC_CNT(tail, head, stream->oa_buffer.vma->size);
}
static u32 _rewind_tail(struct i915_perf_stream *stream, u32 relative_hw_tail,
u32 rewind_delta)
{
return CIRC_CNT(relative_hw_tail, rewind_delta, stream->oa_buffer.vma->size);
}
Note that for power-of-2 sizes the two functions above are identical but we
should keep them separate for clarity (as is done in the patch) since they
are serving two different functions in the OA code.
Also another assumption in the code seems to be:
stream->oa_buffer.vma->size == OA_BUFFER_SIZE
which I am pretty sure will not hold for arbitrary non-power-of-2 OA buffer
sizes? So we might as well stick with power-of-2 sizes and change later in
a separate patch only if needed?
Most changes here are related to the OA buffer size issue and that is
specific to xehpsdv where the size is not a power of 2. I am thinking of
dropping these changes in the next revision since DG2 is fixed and OA
buffer sizes are power of 2.
Thanks,
Umesh
Thanks.
--
Ashutosh
