On Tue, 03 Sep 2019, Animesh Manna <animesh.ma...@intel.com> wrote: > Hi, > > > On 8/30/2019 7:05 PM, Jani Nikula wrote: >> On Fri, 30 Aug 2019, Animesh Manna <animesh.ma...@intel.com> wrote: >>> This patch adds a function, which will internally get the gem buffer >>> for DSB engine. The GEM buffer is from global GTT, and is mapped into >>> CPU domain, contains the data + opcode to be feed to DSB engine. >>> >>> v1: Initial version. >>> >>> v2: >>> - removed some unwanted code. (Chris) >>> - Used i915_gem_object_create_internal instead of _shmem. (Chris) >>> - cmd_buf_tail removed and can be derived through vma object. (Chris) >>> >>> v3: vma realeased if i915_gem_object_pin_map() failed. (Shashank) >>> >>> Cc: Imre Deak <imre.d...@intel.com> >>> Cc: Michel Thierry <michel.thie...@intel.com> >>> Cc: Jani Nikula <jani.nik...@intel.com> >>> Cc: Rodrigo Vivi <rodrigo.v...@intel.com> >>> Signed-off-by: Animesh Manna <animesh.ma...@intel.com> >>> --- >>> drivers/gpu/drm/i915/Makefile | 1 + >>> .../drm/i915/display/intel_display_types.h | 3 + >>> drivers/gpu/drm/i915/display/intel_dsb.c | 83 +++++++++++++++++++ >>> drivers/gpu/drm/i915/display/intel_dsb.h | 31 +++++++ >>> drivers/gpu/drm/i915/i915_drv.h | 1 + >>> 5 files changed, 119 insertions(+) >>> create mode 100644 drivers/gpu/drm/i915/display/intel_dsb.c >>> create mode 100644 drivers/gpu/drm/i915/display/intel_dsb.h >>> >>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile >>> index 658b930d34a8..6313e7b4bd78 100644 >>> --- a/drivers/gpu/drm/i915/Makefile >>> +++ b/drivers/gpu/drm/i915/Makefile >>> @@ -172,6 +172,7 @@ i915-y += \ >>> display/intel_display_power.o \ >>> display/intel_dpio_phy.o \ >>> display/intel_dpll_mgr.o \ >>> + display/intel_dsb.o \ >>> display/intel_fbc.o \ >>> display/intel_fifo_underrun.o \ >>> display/intel_frontbuffer.o \ >>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h >>> b/drivers/gpu/drm/i915/display/intel_display_types.h >>> index 61277a87dbe7..da36867189cb 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h >>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h >>> @@ -1033,6 +1033,9 @@ struct intel_crtc { >>> >>> /* scalers available on this crtc */ >>> int num_scalers; >>> + >>> + /* per pipe DSB related info */ >>> + struct intel_dsb dsb[MAX_DSB_PER_PIPE]; >>> }; >>> >>> struct intel_plane { >>> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c >>> b/drivers/gpu/drm/i915/display/intel_dsb.c >>> new file mode 100644 >>> index 000000000000..007ef13481d5 >>> --- /dev/null >>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c >>> @@ -0,0 +1,83 @@ >>> +// SPDX-License-Identifier: MIT >>> +/* >>> + * Copyright © 2019 Intel Corporation >>> + * >>> + */ >>> + >>> +#include "i915_drv.h" >>> +#include "intel_display_types.h" >>> + >>> +#define DSB_BUF_SIZE (2 * PAGE_SIZE) >>> + >>> +struct intel_dsb * >>> +intel_dsb_get(struct intel_crtc *crtc) >>> +{ >>> + struct drm_device *dev = crtc->base.dev; >>> + struct drm_i915_private *i915 = to_i915(dev); >>> + struct drm_i915_gem_object *obj; >>> + struct i915_vma *vma; >>> + struct intel_dsb *dsb; >>> + intel_wakeref_t wakeref; >>> + int i; >>> + >>> + for (i = 0; i < MAX_DSB_PER_PIPE; i++) >>> + if (!crtc->dsb[i].cmd_buf) >>> + break; >>> + >>> + if (WARN_ON(i == MAX_DSB_PER_PIPE)) >>> + return NULL; >>> + >>> + dsb = &crtc->dsb[i]; >>> + dsb->id = i; >>> + dsb->crtc = crtc; >>> + if (!HAS_DSB(i915)) >>> + return dsb; >> Why do you go through any of the trouble if there is no DSB? Just early >> return NULL, and make the write functions handle NULL dsb pointer. > > Handing NULL dsb pointer in write function is easy, but getting dev_priv > which is needed for I915_WRITE is tricky with the following function > prototypes which were agreed (I can understand it was discussed long > back, so added below). Good to get your suggestion here. > > struct intel_dsb * intel_dsb_get(struct intel_crtc *crtc); > void intel_dsb_put(struct intel_dsb *dsb); > void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val); > void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg, > u32 val); > void intel_dsb_commit(struct intel_dsb *dsb);
Right, silly me. Need to ensure you have a fallback dsb that always has the crtc in place to figure out i915. Both are a bit meh, but I lean towards the latter. BR, Jani. > > Regards, > Animesh > > >> >>> + >>> + wakeref = intel_runtime_pm_get(&i915->runtime_pm); >>> + >>> + obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE); >>> + if (IS_ERR(obj)) >>> + goto err; >>> + >>> + mutex_lock(&i915->drm.struct_mutex); >>> + vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, PIN_MAPPABLE); >>> + mutex_unlock(&i915->drm.struct_mutex); >>> + if (IS_ERR(vma)) { >>> + DRM_ERROR("Vma creation failed.\n"); >>> + i915_gem_object_put(obj); >>> + goto err; >>> + } >>> + >>> + dsb->cmd_buf = i915_gem_object_pin_map(vma->obj, I915_MAP_WC); >>> + if (IS_ERR(dsb->cmd_buf)) { >>> + DRM_ERROR("Command buffer creation failed.\n"); >>> + i915_vma_unpin_and_release(&vma, 0); >>> + dsb->cmd_buf = NULL; >>> + goto err; >>> + } >>> + dsb->vma = vma; >>> + >>> +err: >>> + intel_runtime_pm_put(&i915->runtime_pm, wakeref); >>> + return dsb; >>> +} >>> + >>> +void intel_dsb_put(struct intel_dsb *dsb) >>> +{ >>> + struct intel_crtc *crtc; >>> + struct drm_i915_private *i915; >>> + >>> + if (!dsb) >>> + return; >>> + >>> + crtc = dsb->crtc; >>> + i915 = to_i915(crtc->base.dev); >>> + >>> + if (dsb->cmd_buf) { >>> + mutex_lock(&i915->drm.struct_mutex); >>> + i915_gem_object_unpin_map(dsb->vma->obj); >>> + i915_vma_unpin_and_release(&dsb->vma, 0); >>> + dsb->cmd_buf = NULL; >>> + mutex_unlock(&i915->drm.struct_mutex); >>> + } >>> +} >>> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h >>> b/drivers/gpu/drm/i915/display/intel_dsb.h >>> new file mode 100644 >>> index 000000000000..4a4091cadc1e >>> --- /dev/null >>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.h >>> @@ -0,0 +1,31 @@ >>> +/* SPDX-License-Identifier: MIT >>> + * >>> + * Copyright © 2019 Intel Corporation >>> + */ >>> + >>> +#ifndef _INTEL_DSB_H >>> +#define _INTEL_DSB_H >>> + >>> +struct intel_crtc; >>> +struct i915_vma; >>> + >>> +enum dsb_id { >>> + INVALID_DSB = -1, >>> + DSB1, >>> + DSB2, >>> + DSB3, >>> + MAX_DSB_PER_PIPE >>> +}; >>> + >>> +struct intel_dsb { >>> + struct intel_crtc *crtc; >>> + enum dsb_id id; >>> + u32 *cmd_buf; >>> + struct i915_vma *vma; >>> +}; >>> + >>> +struct intel_dsb * >>> +intel_dsb_get(struct intel_crtc *crtc); >>> +void intel_dsb_put(struct intel_dsb *dsb); >>> + >>> +#endif >>> diff --git a/drivers/gpu/drm/i915/i915_drv.h >>> b/drivers/gpu/drm/i915/i915_drv.h >>> index 804bfe7aec2b..7aa11e3dd413 100644 >>> --- a/drivers/gpu/drm/i915/i915_drv.h >>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>> @@ -67,6 +67,7 @@ >>> #include "display/intel_display.h" >>> #include "display/intel_display_power.h" >>> #include "display/intel_dpll_mgr.h" >>> +#include "display/intel_dsb.h" >>> #include "display/intel_frontbuffer.h" >>> #include "display/intel_gmbus.h" >>> #include "display/intel_opregion.h" > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx