mbox series

[v5,0/5] Use drm_clflush* instead of clflush

Message ID 20220204163711.439403-1-michael.cheng@intel.com (mailing list archive)
Headers show
Series Use drm_clflush* instead of clflush | expand

Message

Michael Cheng Feb. 4, 2022, 4:37 p.m. UTC
This patch series re-work a few i915 functions to use drm_clflush_virt_range
instead of calling clflush or clflushopt directly. This will prevent errors
when building for non-x86 architectures.

v2: s/PAGE_SIZE/sizeof(value) for Re-work intel_write_status_page and added
more patches to convert additional clflush/clflushopt to use drm_clflush*.
(Michael Cheng)

v3: Drop invalidate_csb_entries and directly invoke drm_clflush_virt_ran

v4: Remove extra memory barriers 

v5: s/cache_clflush_range/drm_clflush_virt_range

Michael Cheng (5):
  drm/i915/gt: Re-work intel_write_status_page
  drm/i915/gt: Drop invalidate_csb_entries
  drm/i915/gt: Re-work reset_csb
  drm/i915/: Re-work clflush_write32
  drm/i915/gt: replace cache_clflush_range

 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  8 +++-----
 drivers/gpu/drm/i915/gt/gen8_ppgtt.c          | 12 ++++++------
 drivers/gpu/drm/i915/gt/intel_engine.h        | 13 ++++---------
 .../drm/i915/gt/intel_execlists_submission.c  | 19 ++++++-------------
 drivers/gpu/drm/i915/gt/intel_gtt.c           |  2 +-
 drivers/gpu/drm/i915/gt/intel_ppgtt.c         |  2 +-
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c |  2 +-
 7 files changed, 22 insertions(+), 36 deletions(-)

Comments

Tvrtko Ursulin Feb. 7, 2022, 12:03 p.m. UTC | #1
On 04/02/2022 16:37, Michael Cheng wrote:
> This patch series re-work a few i915 functions to use drm_clflush_virt_range
> instead of calling clflush or clflushopt directly. This will prevent errors
> when building for non-x86 architectures.
> 
> v2: s/PAGE_SIZE/sizeof(value) for Re-work intel_write_status_page and added
> more patches to convert additional clflush/clflushopt to use drm_clflush*.
> (Michael Cheng)
> 
> v3: Drop invalidate_csb_entries and directly invoke drm_clflush_virt_ran
> 
> v4: Remove extra memory barriers
> 
> v5: s/cache_clflush_range/drm_clflush_virt_range

Is anyone interested in this story noticing my open? I will repeat:

How about we add i915_clflush_virt_range as static inline and by doing 
so avoid adding function calls to code paths which are impossible on Arm 
builds? Case in point relocations, probably execlists backend as well.

Downside would be effectively duplicating drm_clfush_virt_range code. 
But for me, (Also considering no other driver calls it so why it is 
there? Should it be deleted?), that would be okay.

Regards,

Tvrtko

> Michael Cheng (5):
>    drm/i915/gt: Re-work intel_write_status_page
>    drm/i915/gt: Drop invalidate_csb_entries
>    drm/i915/gt: Re-work reset_csb
>    drm/i915/: Re-work clflush_write32
>    drm/i915/gt: replace cache_clflush_range
> 
>   .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  8 +++-----
>   drivers/gpu/drm/i915/gt/gen8_ppgtt.c          | 12 ++++++------
>   drivers/gpu/drm/i915/gt/intel_engine.h        | 13 ++++---------
>   .../drm/i915/gt/intel_execlists_submission.c  | 19 ++++++-------------
>   drivers/gpu/drm/i915/gt/intel_gtt.c           |  2 +-
>   drivers/gpu/drm/i915/gt/intel_ppgtt.c         |  2 +-
>   .../gpu/drm/i915/gt/uc/intel_guc_submission.c |  2 +-
>   7 files changed, 22 insertions(+), 36 deletions(-)
>
Jani Nikula Feb. 7, 2022, 12:44 p.m. UTC | #2
On Mon, 07 Feb 2022, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> On 04/02/2022 16:37, Michael Cheng wrote:
>> This patch series re-work a few i915 functions to use drm_clflush_virt_range
>> instead of calling clflush or clflushopt directly. This will prevent errors
>> when building for non-x86 architectures.
>> 
>> v2: s/PAGE_SIZE/sizeof(value) for Re-work intel_write_status_page and added
>> more patches to convert additional clflush/clflushopt to use drm_clflush*.
>> (Michael Cheng)
>> 
>> v3: Drop invalidate_csb_entries and directly invoke drm_clflush_virt_ran
>> 
>> v4: Remove extra memory barriers
>> 
>> v5: s/cache_clflush_range/drm_clflush_virt_range
>
> Is anyone interested in this story noticing my open? I will repeat:
>
> How about we add i915_clflush_virt_range as static inline and by doing 
> so avoid adding function calls to code paths which are impossible on Arm 
> builds? Case in point relocations, probably execlists backend as well.
>
> Downside would be effectively duplicating drm_clfush_virt_range code. 
> But for me, (Also considering no other driver calls it so why it is 
> there? Should it be deleted?), that would be okay.

Keep it simple first, optimize later if necessary?

BR,
Jani.


>
> Regards,
>
> Tvrtko
>
>> Michael Cheng (5):
>>    drm/i915/gt: Re-work intel_write_status_page
>>    drm/i915/gt: Drop invalidate_csb_entries
>>    drm/i915/gt: Re-work reset_csb
>>    drm/i915/: Re-work clflush_write32
>>    drm/i915/gt: replace cache_clflush_range
>> 
>>   .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  8 +++-----
>>   drivers/gpu/drm/i915/gt/gen8_ppgtt.c          | 12 ++++++------
>>   drivers/gpu/drm/i915/gt/intel_engine.h        | 13 ++++---------
>>   .../drm/i915/gt/intel_execlists_submission.c  | 19 ++++++-------------
>>   drivers/gpu/drm/i915/gt/intel_gtt.c           |  2 +-
>>   drivers/gpu/drm/i915/gt/intel_ppgtt.c         |  2 +-
>>   .../gpu/drm/i915/gt/uc/intel_guc_submission.c |  2 +-
>>   7 files changed, 22 insertions(+), 36 deletions(-)
>>
Tvrtko Ursulin Feb. 8, 2022, 9:02 a.m. UTC | #3
On 07/02/2022 12:44, Jani Nikula wrote:
> On Mon, 07 Feb 2022, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>> On 04/02/2022 16:37, Michael Cheng wrote:
>>> This patch series re-work a few i915 functions to use drm_clflush_virt_range
>>> instead of calling clflush or clflushopt directly. This will prevent errors
>>> when building for non-x86 architectures.
>>>
>>> v2: s/PAGE_SIZE/sizeof(value) for Re-work intel_write_status_page and added
>>> more patches to convert additional clflush/clflushopt to use drm_clflush*.
>>> (Michael Cheng)
>>>
>>> v3: Drop invalidate_csb_entries and directly invoke drm_clflush_virt_ran
>>>
>>> v4: Remove extra memory barriers
>>>
>>> v5: s/cache_clflush_range/drm_clflush_virt_range
>>
>> Is anyone interested in this story noticing my open? I will repeat:
>>
>> How about we add i915_clflush_virt_range as static inline and by doing
>> so avoid adding function calls to code paths which are impossible on Arm
>> builds? Case in point relocations, probably execlists backend as well.
>>
>> Downside would be effectively duplicating drm_clfush_virt_range code.
>> But for me, (Also considering no other driver calls it so why it is
>> there? Should it be deleted?), that would be okay.
> 
> Keep it simple first, optimize later if necessary?

I don't think it would ever happen unless done from the start. :/

Regards,

Tvrtko