[Intel-gfx] [PATCH 0/7] drm/i915/display: split out verifation, compare and dump from intel_display.c

[Jani Nikula]

On Wed, 15 Jun 2022, Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> On Wed, Jun 15, 2022 at 03:47:54PM +0300, Jani Nikula wrote:
>> The state verification and pipe config comparison/dumping are fairly
>> isolated pieces of code within intel_display.c. Move them to separate
>> files in a long overdue cleanup.
>> 
>> Jani Nikula (7):
>>   drm/i915/wm: move wm state verification to intel_pm.c
>>   drm/i915/dpll: move shared dpll state verification to intel_dpll_mgr.c
>>   drm/i915/mpllb: use I915_STATE_WARN() for state mismatch warnings
>>   drm/i915/mpllb: move mpllb state check to intel_snps_phy.c
>
> I'd perhaps go for foo_state_verify() naming convention. Would
> match the foo_state_dump() naming convention I suggested
> for the dumping stuff.

Roger.

>
> Apart from that these ^ four are
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
>>   drm/i915/display: split out modeset verification code
>
> I really hate some of that code. I guess hiding it is one option :P
> This one ^ is
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Yeah, there's some weird stuff. For example we only call
verify_encoder_state() only to verify it's disabled.

>
>>   drm/i915/display: split out pipe config compare to a separate file
>
> Not entirely sure I like moving this one. The fastset stuff
> within needs to stay in sync with the fastset codepaths which
> are in intel_display.c. Not sure if we risk more bugs if it's
> too well hidden...

Mixed feelings. The problem remains, the file is still too damn big, and
it's hard to draw the lines what to extract. Maybe all the modeset code
needs to be lifted, along with the config compare, but then I think that
has too many dependencies to axe out cleanly. Damned if you do, damned
if you don't.

BR,
Jani.


>
>>   drm/i915/display: split out pipe config dump to a separate file
>> 
>>  drivers/gpu/drm/i915/Makefile                 |    3 +
>>  drivers/gpu/drm/i915/display/intel_display.c  | 1373 +----------------
>>  drivers/gpu/drm/i915/display/intel_display.h  |    3 +
>>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c |   88 ++
>>  drivers/gpu/drm/i915/display/intel_dpll_mgr.h |    5 +
>>  .../drm/i915/display/intel_modeset_verify.c   |  247 +++
>>  .../drm/i915/display/intel_modeset_verify.h   |   21 +
>>  .../i915/display/intel_pipe_config_compare.c  |  581 +++++++
>>  .../i915/display/intel_pipe_config_compare.h  |   17 +
>>  .../drm/i915/display/intel_pipe_config_dump.c |  314 ++++
>>  .../drm/i915/display/intel_pipe_config_dump.h |   16 +
>>  drivers/gpu/drm/i915/display/intel_snps_phy.c |   43 +
>>  drivers/gpu/drm/i915/display/intel_snps_phy.h |    5 +-
>>  drivers/gpu/drm/i915/intel_pm.c               |  138 +-
>>  drivers/gpu/drm/i915/intel_pm.h               |   14 +-
>>  15 files changed, 1482 insertions(+), 1386 deletions(-)
>>  create mode 100644 drivers/gpu/drm/i915/display/intel_modeset_verify.c
>>  create mode 100644 drivers/gpu/drm/i915/display/intel_modeset_verify.h
>>  create mode 100644 drivers/gpu/drm/i915/display/intel_pipe_config_compare.c
>>  create mode 100644 drivers/gpu/drm/i915/display/intel_pipe_config_compare.h
>>  create mode 100644 drivers/gpu/drm/i915/display/intel_pipe_config_dump.c
>>  create mode 100644 drivers/gpu/drm/i915/display/intel_pipe_config_dump.h
>> 
>> -- 
>> 2.30.2

-- 
Jani Nikula, Intel Open Source Graphics Center