Message ID | 20190830124533.26573-3-animesh.manna@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | DSB enablement. | expand |
On Fri, 30 Aug 2019, Animesh Manna <animesh.manna@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.deak@intel.com> > Cc: Michel Thierry <michel.thierry@intel.com> > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Animesh Manna <animesh.manna@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. > + > + 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"
> -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Jani > Nikula > Sent: Friday, August 30, 2019 7:06 PM > To: Manna, Animesh <animesh.manna@intel.com>; intel-gfx@lists.freedesktop.org > Cc: Thierry, Michel <michel.thierry@intel.com> > Subject: Re: [Intel-gfx] [PATCH v4 02/10] drm/i915/dsb: DSB context creation. > > On Fri, 30 Aug 2019, Animesh Manna <animesh.manna@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.deak@intel.com> > > Cc: Michel Thierry <michel.thierry@intel.com> > > Cc: Jani Nikula <jani.nikula@intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Signed-off-by: Animesh Manna <animesh.manna@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. > 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi, On 8/30/2019 7:05 PM, Jani Nikula wrote: > On Fri, 30 Aug 2019, Animesh Manna <animesh.manna@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.deak@intel.com> >> Cc: Michel Thierry <michel.thierry@intel.com> >> Cc: Jani Nikula <jani.nikula@intel.com> >> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >> Signed-off-by: Animesh Manna <animesh.manna@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); 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"
On Tue, 03 Sep 2019, Animesh Manna <animesh.manna@intel.com> wrote: > Hi, > > > On 8/30/2019 7:05 PM, Jani Nikula wrote: >> On Fri, 30 Aug 2019, Animesh Manna <animesh.manna@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.deak@intel.com> >>> Cc: Michel Thierry <michel.thierry@intel.com> >>> Cc: Jani Nikula <jani.nikula@intel.com> >>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >>> Signed-off-by: Animesh Manna <animesh.manna@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
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; + + 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"
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@intel.com> Cc: Michel Thierry <michel.thierry@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Animesh Manna <animesh.manna@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