Re: [Intel-gfx] [RFC PATCH 0/4] Framework for display parameters

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 05/10/2023 13:52, Jani Nikula wrote:
> On Thu, 05 Oct 2023, Jouni Högander <jouni.hogander@intel.com> wrote:
>> Currently all module parameters are handled by i915_param.c/h. This
>> is a problem for display parameters when Xe driver is used.
>>
>> This patch set adds a mechanism to add parameters specific to the
>> display. This is mainly copied from existing i915 parameters
>> implementation with some naming changes and taking into account
>> varying driver name.
>>
>> Also fbc and psr related parameter are moved as an example.
> 
> Added some more Cc's as FYI.
> 
> It's not perfect, but I'm okay with the general approach. I've hoped for
> a beautiful solution to the problem, but hopes don't turn into code.
> 
> I'll reply with some bikeshedding comments to the individual patches.

Looks fine to me.

If one day display becomes a separate .ko and we maybe allow not 
building it for headless then the interface is a bit too intimate, but 
until then it looks like it will work fine.

Regards,

Tvrtko

> 
> BR,
> Jani.
> 
> 
>>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Uma Shankar <uma.shankar@intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>
>> Jouni Högander (4):
>>    drm/i915/display: Add framework to add parameters specific to display
>>    drm/i915/display: Dump also display parameters into GPU error dump
>>    drm/i915/display: Move enable_fbc module parameter under display
>>    drm/i915/display: Move psr related module parameters under display
>>
>>   drivers/gpu/drm/i915/Makefile                 |   2 +
>>   drivers/gpu/drm/i915/display/i9xx_wm.c        |   2 +-
>>   .../gpu/drm/i915/display/intel_display_core.h |   2 +
>>   .../drm/i915/display/intel_display_debugfs.c  |   2 +
>>   .../display/intel_display_debugfs_params.c    | 239 ++++++++++++++++++
>>   .../display/intel_display_debugfs_params.h    |  14 +
>>   .../drm/i915/display/intel_display_device.c   |   8 +
>>   .../drm/i915/display/intel_display_device.h   |   1 +
>>   .../drm/i915/display/intel_display_params.c   | 166 ++++++++++++
>>   .../drm/i915/display/intel_display_params.h   |  65 +++++
>>   drivers/gpu/drm/i915/display/intel_fbc.c      |  10 +-
>>   drivers/gpu/drm/i915/display/intel_psr.c      |  14 +-
>>   drivers/gpu/drm/i915/i915_driver.c            |   2 +
>>   drivers/gpu/drm/i915/i915_gpu_error.c         |   3 +
>>   drivers/gpu/drm/i915/i915_gpu_error.h         |   2 +
>>   drivers/gpu/drm/i915/i915_params.c            |  19 --
>>   drivers/gpu/drm/i915/i915_params.h            |   4 -
>>   17 files changed, 519 insertions(+), 36 deletions(-)
>>   create mode 100644 drivers/gpu/drm/i915/display/intel_display_debugfs_params.c
>>   create mode 100644 drivers/gpu/drm/i915/display/intel_display_debugfs_params.h
>>   create mode 100644 drivers/gpu/drm/i915/display/intel_display_params.c
>>   create mode 100644 drivers/gpu/drm/i915/display/intel_display_params.h
>