| Message ID | 20221020120457.19528-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/i915: Clean up crtc state flag checks | expand |
On Thu, 20 Oct 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Some cleanups for checking whether the crtc was flagged for > modesets/fastsets/color update. I wish we could avoid piling more static inlines in intel_display_types.h, but the clarity added here is great. On the series, Reviewed-by: Jani Nikula <jani.nikula@intel.com> > > Ville Syrjälä (4): > drm/i915: Introduce intel_crtc_needs_fastset() > drm/i915: Remove some local 'mode_changed' bools > drm/i915: Don't flag both full modeset and fastset at the same time > drm/i915: Introduce intel_crtc_needs_color_update() > > drivers/gpu/drm/i915/display/hsw_ips.c | 8 ++-- > drivers/gpu/drm/i915/display/intel_crtc.c | 3 +- > drivers/gpu/drm/i915/display/intel_cursor.c | 6 ++- > drivers/gpu/drm/i915/display/intel_display.c | 46 +++++++++---------- > .../drm/i915/display/intel_display_types.h | 14 ++++++ > .../drm/i915/display/intel_modeset_verify.c | 3 +- > 6 files changed, 46 insertions(+), 34 deletions(-)
On Thu, Oct 20, 2022 at 04:45:55PM +0300, Jani Nikula wrote: > On Thu, 20 Oct 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Some cleanups for checking whether the crtc was flagged for > > modesets/fastsets/color update. > > I wish we could avoid piling more static inlines in > intel_display_types.h, but the clarity added here is great. I mainly put them there since the first one was already there. Dunno if the function call overhead would really be measurable, even though we do use these a lot. Should measure it on some real slouch of a machine I guess. > > On the series, > > Reviewed-by: Jani Nikula <jani.nikula@intel.com> Thanks. > > > > > Ville Syrjälä (4): > > drm/i915: Introduce intel_crtc_needs_fastset() > > drm/i915: Remove some local 'mode_changed' bools > > drm/i915: Don't flag both full modeset and fastset at the same time > > drm/i915: Introduce intel_crtc_needs_color_update() > > > > drivers/gpu/drm/i915/display/hsw_ips.c | 8 ++-- > > drivers/gpu/drm/i915/display/intel_crtc.c | 3 +- > > drivers/gpu/drm/i915/display/intel_cursor.c | 6 ++- > > drivers/gpu/drm/i915/display/intel_display.c | 46 +++++++++---------- > > .../drm/i915/display/intel_display_types.h | 14 ++++++ > > .../drm/i915/display/intel_modeset_verify.c | 3 +- > > 6 files changed, 46 insertions(+), 34 deletions(-) > > -- > Jani Nikula, Intel Open Source Graphics Center
On Thu, 20 Oct 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Thu, Oct 20, 2022 at 04:45:55PM +0300, Jani Nikula wrote: >> On Thu, 20 Oct 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com> >> > >> > Some cleanups for checking whether the crtc was flagged for >> > modesets/fastsets/color update. >> >> I wish we could avoid piling more static inlines in >> intel_display_types.h, but the clarity added here is great. > > I mainly put them there since the first one was already there. > Dunno if the function call overhead would really be measurable, > even though we do use these a lot. Should measure it on some > real slouch of a machine I guess. Well, I think some things can be static inlines just fine. In particular, static inlines that don't require pulling in *extra* headers are largely just fine. But as soon as you need to look at e.g. struct drm_i915_private, you're toast. The ones added here only need the definition of struct intel_crtc_state which is right there. In this case, I'm more worried about the general bloating of intel_display_types.h. And it's not even all that bad with these patches, just piling more stuff little by little. So I wonder if we could have intel_crtc_state.h defining just struct intel_crtc_state and maybe some things only contained within it. I've found one of the biggest obstacles to splitting more stuff is actually the enums. A lot of places need the last _MAX and _NUM enumerators for defining member array sizes. Ditto with intel_crtc_state. I've toyed with having intel_display_limits.h with just the limits as #defines and separate build checks to match the enum max. I've also toyed with having intel_display_enums.h with just the enums, so we keep single point of truth but avoid including a lot of unnecessary stuff from intel_display.h and intel_display_types.h. Got some patches, but didn't get too far. A lot of random rambling here, but does not impact the patches at hand. BR, Jani. > >> >> On the series, >> >> Reviewed-by: Jani Nikula <jani.nikula@intel.com> > > Thanks. > >> >> > >> > Ville Syrjälä (4): >> > drm/i915: Introduce intel_crtc_needs_fastset() >> > drm/i915: Remove some local 'mode_changed' bools >> > drm/i915: Don't flag both full modeset and fastset at the same time >> > drm/i915: Introduce intel_crtc_needs_color_update() >> > >> > drivers/gpu/drm/i915/display/hsw_ips.c | 8 ++-- >> > drivers/gpu/drm/i915/display/intel_crtc.c | 3 +- >> > drivers/gpu/drm/i915/display/intel_cursor.c | 6 ++- >> > drivers/gpu/drm/i915/display/intel_display.c | 46 +++++++++---------- >> > .../drm/i915/display/intel_display_types.h | 14 ++++++ >> > .../drm/i915/display/intel_modeset_verify.c | 3 +- >> > 6 files changed, 46 insertions(+), 34 deletions(-) >> >> -- >> Jani Nikula, Intel Open Source Graphics Center
From: Ville Syrjälä <ville.syrjala@linux.intel.com> Some cleanups for checking whether the crtc was flagged for modesets/fastsets/color update. Ville Syrjälä (4): drm/i915: Introduce intel_crtc_needs_fastset() drm/i915: Remove some local 'mode_changed' bools drm/i915: Don't flag both full modeset and fastset at the same time drm/i915: Introduce intel_crtc_needs_color_update() drivers/gpu/drm/i915/display/hsw_ips.c | 8 ++-- drivers/gpu/drm/i915/display/intel_crtc.c | 3 +- drivers/gpu/drm/i915/display/intel_cursor.c | 6 ++- drivers/gpu/drm/i915/display/intel_display.c | 46 +++++++++---------- .../drm/i915/display/intel_display_types.h | 14 ++++++ .../drm/i915/display/intel_modeset_verify.c | 3 +- 6 files changed, 46 insertions(+), 34 deletions(-)