| Message ID | 20240826111527.1113622-1-ankit.k.nautiyal@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Consolidation of DSS Control in Separate Files | expand |
On Mon, 26 Aug 2024, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote: > Currently, DSS control is configured from various files; this change aims > to consolidate all DSS-related functionalities, such as display stream > splitting, joining, MSO configuration, and joining configuration, > into one place. > > A new file, intel_dss_regs.h, will store register information, while the > helpers to configure DSS and related stuff will be moved to intel_dss.c > with its corresponding header file intel_dss.h. > Along with this, the helpers related to retrieve information about the > pipe joiners are also moved from intel_display.c to intel_dss. > > Additionally, wherever possible, the drm_i915_private structure is > replaced with the new intel_display structure as part of ongoing efforts > to phase out the old structure. A bunch of nitpicks here and there, but overall I like the direction. Thanks for doing this. I see that the dsi and mso stuff are just thrown in there instead of really "integrated" into anything, but I think that's fine as the first step. I think maybe we do want to have a separate file for joiner stuff like you suggested offline, maybe intel_joiner.[ch]? There's a whole lot of joiner stuff still left in intel_display.[ch] after this. Sooo... anything to do with DSS_CTL regs would go to intel_dss.[ch], anything to do with compression would go to intel_vdsc.[ch], and anything to do with joining would go to intel_joiner.[ch]? Ville? BR, Jani. > > Ankit Nautiyal (12): > drm/i915/display: Move all DSS control registers to a new file > drm/i915/ddi: Move all mso related helpers to a new file > drm/i915/dss: Move to struct intel_display > drm/i915/icl_dsi: Move helpers to configure dsi dual link to intel_dss > drm/i915/vdsc: Rename helper to check if the pipe supports dsc > drm/i915/vdsc: Move all dss stuff in dss files > drm/i915/display: Move dss stuff in intel_dss files > drm/i915/display: Move helper to get joined pipe mask to intel_dss > drm/i915/display: Move helpers for primary joiner to intel_dss > drm/i915/display: Move helper to check for secondary joiner pipe > drm/i915/display: Move helper to get all secondary pipes > drm/i915/display: Move intel_joiner_num_pipes to intel dss > > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/display/icl_dsi.c | 55 +--- > .../gpu/drm/i915/display/intel_atomic_plane.c | 3 +- > .../drm/i915/display/intel_crtc_state_dump.c | 5 +- > drivers/gpu/drm/i915/display/intel_ddi.c | 94 +----- > drivers/gpu/drm/i915/display/intel_display.c | 158 +++------ > drivers/gpu/drm/i915/display/intel_display.h | 4 - > .../drm/i915/display/intel_display_debugfs.c | 3 +- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 +- > drivers/gpu/drm/i915/display/intel_drrs.c | 5 +- > drivers/gpu/drm/i915/display/intel_dss.c | 305 ++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_dss.h | 46 +++ > drivers/gpu/drm/i915/display/intel_dss_regs.h | 49 +++ > .../drm/i915/display/intel_modeset_setup.c | 15 +- > drivers/gpu/drm/i915/display/intel_vdsc.c | 74 +---- > drivers/gpu/drm/i915/display/intel_vdsc.h | 2 +- > .../gpu/drm/i915/display/intel_vdsc_regs.h | 38 --- > drivers/gpu/drm/xe/Makefile | 1 + > 18 files changed, 497 insertions(+), 368 deletions(-) > create mode 100644 drivers/gpu/drm/i915/display/intel_dss.c > create mode 100644 drivers/gpu/drm/i915/display/intel_dss.h > create mode 100644 drivers/gpu/drm/i915/display/intel_dss_regs.h
On 8/26/2024 6:04 PM, Jani Nikula wrote: > On Mon, 26 Aug 2024, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote: >> Currently, DSS control is configured from various files; this change aims >> to consolidate all DSS-related functionalities, such as display stream >> splitting, joining, MSO configuration, and joining configuration, >> into one place. >> >> A new file, intel_dss_regs.h, will store register information, while the >> helpers to configure DSS and related stuff will be moved to intel_dss.c >> with its corresponding header file intel_dss.h. >> Along with this, the helpers related to retrieve information about the >> pipe joiners are also moved from intel_display.c to intel_dss. >> >> Additionally, wherever possible, the drm_i915_private structure is >> replaced with the new intel_display structure as part of ongoing efforts >> to phase out the old structure. > A bunch of nitpicks here and there, but overall I like the > direction. Thanks for doing this. Thanks Jani for the review comments and suggestions. I agree to the changes and corrections suggested and will address them in next revision. > > I see that the dsi and mso stuff are just thrown in there instead of > really "integrated" into anything, but I think that's fine as the first > step. I agree. I was thinking if we should have a separate struct intel_dss with members for different features, which we configure in compute_config phase and later write dss_ctl register at one place? Same for reading and get_config. If it makes sense, perhaps can take it as a separate series. > > I think maybe we do want to have a separate file for joiner stuff like > you suggested offline, maybe intel_joiner.[ch]? There's a whole lot of > joiner stuff still left in intel_display.[ch] after this. > > Sooo... anything to do with DSS_CTL regs would go to intel_dss.[ch], > anything to do with compression would go to intel_vdsc.[ch], and > anything to do with joining would go to intel_joiner.[ch]? Yes right. The new joiner file can house all the helpers for retrieving primary/secondary joiner_pipes along with helpers to modify src_size, timings etc. Thanks again for the comments! Regards, Ankit > > Ville? > > > BR, > Jani. > > >> Ankit Nautiyal (12): >> drm/i915/display: Move all DSS control registers to a new file >> drm/i915/ddi: Move all mso related helpers to a new file >> drm/i915/dss: Move to struct intel_display >> drm/i915/icl_dsi: Move helpers to configure dsi dual link to intel_dss >> drm/i915/vdsc: Rename helper to check if the pipe supports dsc >> drm/i915/vdsc: Move all dss stuff in dss files >> drm/i915/display: Move dss stuff in intel_dss files >> drm/i915/display: Move helper to get joined pipe mask to intel_dss >> drm/i915/display: Move helpers for primary joiner to intel_dss >> drm/i915/display: Move helper to check for secondary joiner pipe >> drm/i915/display: Move helper to get all secondary pipes >> drm/i915/display: Move intel_joiner_num_pipes to intel dss >> >> drivers/gpu/drm/i915/Makefile | 1 + >> drivers/gpu/drm/i915/display/icl_dsi.c | 55 +--- >> .../gpu/drm/i915/display/intel_atomic_plane.c | 3 +- >> .../drm/i915/display/intel_crtc_state_dump.c | 5 +- >> drivers/gpu/drm/i915/display/intel_ddi.c | 94 +----- >> drivers/gpu/drm/i915/display/intel_display.c | 158 +++------ >> drivers/gpu/drm/i915/display/intel_display.h | 4 - >> .../drm/i915/display/intel_display_debugfs.c | 3 +- >> drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 +- >> drivers/gpu/drm/i915/display/intel_drrs.c | 5 +- >> drivers/gpu/drm/i915/display/intel_dss.c | 305 ++++++++++++++++++ >> drivers/gpu/drm/i915/display/intel_dss.h | 46 +++ >> drivers/gpu/drm/i915/display/intel_dss_regs.h | 49 +++ >> .../drm/i915/display/intel_modeset_setup.c | 15 +- >> drivers/gpu/drm/i915/display/intel_vdsc.c | 74 +---- >> drivers/gpu/drm/i915/display/intel_vdsc.h | 2 +- >> .../gpu/drm/i915/display/intel_vdsc_regs.h | 38 --- >> drivers/gpu/drm/xe/Makefile | 1 + >> 18 files changed, 497 insertions(+), 368 deletions(-) >> create mode 100644 drivers/gpu/drm/i915/display/intel_dss.c >> create mode 100644 drivers/gpu/drm/i915/display/intel_dss.h >> create mode 100644 drivers/gpu/drm/i915/display/intel_dss_regs.h