On Thu, 08 Sep 2022, Ville Syrjälä <[email protected]> wrote: > On Thu, Sep 08, 2022 at 07:57:02PM +0300, Jani Nikula wrote: >> struct intel_dsb can be an opaque type, hidden in intel_dsb.c. Make it >> so. Reduce related includes while at it. >> >> Signed-off-by: Jani Nikula <[email protected]> > > One thing I was mildly worried about with dsb is the cost > of creating the batch (updating LUTs involves writing some > thousands of dwords). So I was pondering whether that should > be inlined as opposed to being a function call per dword. > But as it stands it's already a function call, and > I've not actually measured how fast/slow it really is. > So can't really argue against this sort of stuff, for the > moment at least :)
I'm also on a mission to kill useless static inlines. ;) Anything that requires pulling in additional headers or exposing the guts of the implementation are suspect and need proper justification. > Reviewed-by: Ville Syrjälä <[email protected]> Thanks, Jani. > >> --- >> drivers/gpu/drm/i915/display/intel_color.c | 1 + >> drivers/gpu/drm/i915/display/intel_display.c | 1 + >> drivers/gpu/drm/i915/display/intel_dsb.c | 30 ++++++++++++++++++++ >> drivers/gpu/drm/i915/display/intel_dsb.h | 28 ------------------ >> drivers/gpu/drm/i915/i915_drv.h | 1 - >> 5 files changed, 32 insertions(+), 29 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_color.c >> b/drivers/gpu/drm/i915/display/intel_color.c >> index ed98c732b24e..6bda4274eae9 100644 >> --- a/drivers/gpu/drm/i915/display/intel_color.c >> +++ b/drivers/gpu/drm/i915/display/intel_color.c >> @@ -26,6 +26,7 @@ >> #include "intel_de.h" >> #include "intel_display_types.h" >> #include "intel_dpll.h" >> +#include "intel_dsb.h" >> #include "vlv_dsi_pll.h" >> >> struct intel_color_funcs { >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c >> b/drivers/gpu/drm/i915/display/intel_display.c >> index 2b6bb5ee7698..296cbcd1352c 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display.c >> +++ b/drivers/gpu/drm/i915/display/intel_display.c >> @@ -91,6 +91,7 @@ >> #include "intel_dmc.h" >> #include "intel_dp_link_training.h" >> #include "intel_dpt.h" >> +#include "intel_dsb.h" >> #include "intel_fbc.h" >> #include "intel_fbdev.h" >> #include "intel_fdi.h" >> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c >> b/drivers/gpu/drm/i915/display/intel_dsb.c >> index c4affcb216fd..fc9c3e41c333 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dsb.c >> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c >> @@ -9,6 +9,36 @@ >> #include "i915_drv.h" >> #include "intel_de.h" >> #include "intel_display_types.h" >> +#include "intel_dsb.h" >> + >> +struct i915_vma; >> + >> +enum dsb_id { >> + INVALID_DSB = -1, >> + DSB1, >> + DSB2, >> + DSB3, >> + MAX_DSB_PER_PIPE >> +}; >> + >> +struct intel_dsb { >> + enum dsb_id id; >> + u32 *cmd_buf; >> + struct i915_vma *vma; >> + >> + /* >> + * free_pos will point the first free entry position >> + * and help in calculating tail of command buffer. >> + */ >> + int free_pos; >> + >> + /* >> + * ins_start_offset will help to store start address of the dsb >> + * instuction and help in identifying the batch of auto-increment >> + * register. >> + */ >> + u32 ins_start_offset; >> +}; >> >> #define DSB_BUF_SIZE (2 * PAGE_SIZE) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h >> b/drivers/gpu/drm/i915/display/intel_dsb.h >> index 6cb9c580cdca..74dd2b3343bb 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dsb.h >> +++ b/drivers/gpu/drm/i915/display/intel_dsb.h >> @@ -11,34 +11,6 @@ >> #include "i915_reg_defs.h" >> >> struct intel_crtc_state; >> -struct i915_vma; >> - >> -enum dsb_id { >> - INVALID_DSB = -1, >> - DSB1, >> - DSB2, >> - DSB3, >> - MAX_DSB_PER_PIPE >> -}; >> - >> -struct intel_dsb { >> - enum dsb_id id; >> - u32 *cmd_buf; >> - struct i915_vma *vma; >> - >> - /* >> - * free_pos will point the first free entry position >> - * and help in calculating tail of command buffer. >> - */ >> - int free_pos; >> - >> - /* >> - * ins_start_offset will help to store start address of the dsb >> - * instuction and help in identifying the batch of auto-increment >> - * register. >> - */ >> - u32 ins_start_offset; >> -}; >> >> void intel_dsb_prepare(struct intel_crtc_state *crtc_state); >> void intel_dsb_cleanup(struct intel_crtc_state *crtc_state); >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h >> index 76aad81c014b..be201ba5e9ab 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -38,7 +38,6 @@ >> >> #include "display/intel_display.h" >> #include "display/intel_display_core.h" >> -#include "display/intel_dsb.h" >> >> #include "gem/i915_gem_context_types.h" >> #include "gem/i915_gem_lmem.h" >> -- >> 2.34.1 -- Jani Nikula, Intel Open Source Graphics Center
