> -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of Jani > Nikula > Sent: Friday, August 30, 2019 7:06 PM > To: Manna, Animesh <animesh.manna@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Thierry, Michel <michel.thierry@xxxxxxxxx> > Subject: Re: [PATCH v4 02/10] drm/i915/dsb: DSB context creation. > > On Fri, 30 Aug 2019, Animesh Manna <animesh.manna@xxxxxxxxx> 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.deak@xxxxxxxxx> > > Cc: Michel Thierry <michel.thierry@xxxxxxxxx> > > Cc: Jani Nikula <jani.nikula@xxxxxxxxx> > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > Signed-off-by: Animesh Manna <animesh.manna@xxxxxxxxx> > > --- > > 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. > I agree on this. Even I thought it would be better if we populate the DSB ptr only when we find a valid DSB support on the platform. And the caller of get() can understand this and act accordingly. > > + > > + 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" > > -- > Jani Nikula, Intel Open Source Graphics Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
