| Message ID | 20190827191026.26175-3-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DSB enablement. | expand |
On 8/28/2019 12:40 AM, Animesh Manna wrote: > The function will internally get the gem buffer from global GTT This patch adds a function, which will internally get the gem buffer for DSB engine. > which is mapped in cpu domain to feed the data + opcode for DSB engine. This GEM buffer is from global GTT, and is mapped into CPU domain, contains the data + opcode to be fed 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) > > 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 96514dcc7812..0ab3516b0044 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1026,6 +1026,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..a2845df90573 > --- /dev/null > +++ b/drivers/gpu/drm/i915/display/intel_dsb.c > @@ -0,0 +1,83 @@ > +// SPDX-License-Identifier: MIT > +/* > + * Copyright © 2019 Intel Corporation > + * > + */ > + Any particular reason we don't have the traditional license here, like other files ? > +#include "../i915_drv.h" This is not the way you should include this header. Take example from other files in display folder. > +#include "intel_display_types.h" > + > +#define DSB_BUF_SIZE (2 * PAGE_SIZE) Any particular reason for this 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; > + > + WARN_ON(i >= MAX_DSB_PER_PIPE); Its not possible to have i > MAX_DSB_PER_PIPE as per above logic, so it should be WARN_ON(i == MAX_DSB_PER_PIPE); Also, shouldn't we stop operation here (along with the warning) as clearly the cmd_buf and dsb we are going to get, is garbage ? On negative testing this may cause kernel panic also. > + > + dsb = &crtc->dsb[i]; > + dsb->id = i; > + dsb->crtc = crtc; > + if (!HAS_DSB(i915)) > + return dsb; This check should be the first line in this function. I am not sure why do we want to extract dsb ptr if the platform doesn't even support 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_DEBUG_KMS("Vma creation failed.\n"); > + i915_gem_object_put(obj); Shouldn't gem_object_put() be inside mutex ? > + goto err; > + } > + > + dsb->cmd_buf = i915_gem_object_pin_map(vma->obj, I915_MAP_WC); > + if (IS_ERR(dsb->cmd_buf)) { > + DRM_DEBUG_KMS("Command buffer creation failed.\n"); > + dsb->cmd_buf = NULL; Why don't we have a i915_gem_object_put(obj) here ? > + goto err; > + } > + dsb->vma = vma; > + > +err: I think we should have a i915_gem_object_put(obj) for all error cases here. > + intel_runtime_pm_put(&i915->runtime_pm, wakeref); > + return dsb; Again, we are returning a dsb ptr in all cases. As this patch doesn't have the caller function, I can't understand why are we returning the same ptr in both success or failure cases, but I would prefer a dsb_get() function which returns NULL on failure, dsb* on success. > +} > + > +void intel_dsb_put(struct intel_dsb *dsb) > +{ > + struct intel_crtc *crtc; > + struct drm_i915_private *i915; > + struct i915_vma *vma; > + > + if (!dsb) > + return; So the get() API returns a non-NULL pointer in both success and failure cases, but put() can have a NULL ptr as input. Looks a bit asymmetrical design. Also, we should add API documentation on top of the function now, as we need to understand what can be the return values of the function. > + > + crtc = dsb->crtc; > + i915 = to_i915(crtc->base.dev); > + > + if (dsb->cmd_buf) { > + vma = dsb->vma; > + mutex_lock(&i915->drm.struct_mutex); > + i915_gem_object_unpin_map(vma->obj); > + i915_vma_unpin_and_release(&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 Same as above for license. > + */ > + > +#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 The Bspec says there are 3 DSB DMA engines per pipe, and we have defined MAX_DSB_PER_PIPE = 3 (and the count starts from 0), which means we should be running our loops always < MAX_DSB_PER_PIPE. I would prefer this to be either INVALID_DSB = 0 and DSB3 = MAX_DSB = 3, but this is just a soft requirement, you can pick or ignore. > +}; > + > +struct intel_dsb { > + struct intel_crtc *crtc; > + enum dsb_id id; Note that your INVALID_DSB id is -1 not 0. This means all the intel_dsb structures will be initialized with id=DSB1 not INVALID_DSB. > + u32 *cmd_buf; > + struct i915_vma *vma; > +}; > + This could also be the place to add doc about the get/put functions. > +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 d32cfdb78b74..dd6cfb89b830 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" No external element is using get() / put() functions yet, this change should go into a patch, which is using the functions. - Shashank > #include "display/intel_frontbuffer.h" > #include "display/intel_gmbus.h" > #include "display/intel_opregion.h"
Hi, On 8/28/2019 8:09 PM, Sharma, Shashank wrote: > > On 8/28/2019 12:40 AM, Animesh Manna wrote: >> The function will internally get the gem buffer from global GTT > This patch adds a function, which will internally get the gem buffer > for DSB engine. >> which is mapped in cpu domain to feed the data + opcode for DSB engine. > This GEM buffer is from global GTT, and is mapped into CPU domain, > contains the data + opcode to be fed to DSB engine. ok. >> >> 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) >> >> 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 96514dcc7812..0ab3516b0044 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display_types.h >> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h >> @@ -1026,6 +1026,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..a2845df90573 >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c >> @@ -0,0 +1,83 @@ >> +// SPDX-License-Identifier: MIT >> +/* >> + * Copyright © 2019 Intel Corporation >> + * >> + */ >> + > Any particular reason we don't have the traditional license here, like > other files ? Current trend I hope to use SPDX header otherwise checkpatch is throwing warning/error. >> +#include "../i915_drv.h" > This is not the way you should include this header. Take example from > other files in display folder. #include "i915_drv.h" rt? > >> +#include "intel_display_types.h" >> + >> +#define DSB_BUF_SIZE (2 * PAGE_SIZE) > Any particular reason for this size ? one page is not sufficient and do not want to use too many pages. > >> + >> +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; >> + >> + WARN_ON(i >= MAX_DSB_PER_PIPE); > > Its not possible to have i > MAX_DSB_PER_PIPE as per above logic, so > it should be WARN_ON(i == MAX_DSB_PER_PIPE); ok. > > Also, shouldn't we stop operation here (along with the warning) as > clearly the cmd_buf and dsb we are going to get, is garbage ? On > negative testing this may cause kernel panic also. I feel for erroneous case, as DSB is not handled properly, better to halt the kernel. Good idea to return NULL, will do it. > >> + >> + dsb = &crtc->dsb[i]; >> + dsb->id = i; >> + dsb->crtc = crtc; >> + if (!HAS_DSB(i915)) >> + return dsb; > This check should be the first line in this function. I am not sure > why do we want to extract dsb ptr if the platform doesn't even support > DSB. As per agreed design with Jani we want to replace I915_WRITE with dsp-write api for all platform and for older platform will fallback to i915_WRITE call. >> + >> + 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_DEBUG_KMS("Vma creation failed.\n"); >> + i915_gem_object_put(obj); > Shouldn't gem_object_put() be inside mutex ? Tried to refer existing code to understand the need, may not need it. >> + goto err; >> + } >> + >> + dsb->cmd_buf = i915_gem_object_pin_map(vma->obj, I915_MAP_WC); >> + if (IS_ERR(dsb->cmd_buf)) { >> + DRM_DEBUG_KMS("Command buffer creation failed.\n"); >> + dsb->cmd_buf = NULL; > Why don't we have a i915_gem_object_put(obj) here ? Will add in err-section. > >> + goto err; >> + } >> + dsb->vma = vma; >> + >> +err: > I think we should have a i915_gem_object_put(obj) for all error cases > here. Ok. >> + intel_runtime_pm_put(&i915->runtime_pm, wakeref); >> + return dsb; > Again, we are returning a dsb ptr in all cases. As this patch doesn't > have the caller function, I can't understand why are we returning the > same ptr in both success or failure cases, but I would prefer a > dsb_get() function which returns NULL on failure, dsb* on success. Explained above. >> +} >> + >> +void intel_dsb_put(struct intel_dsb *dsb) >> +{ >> + struct intel_crtc *crtc; >> + struct drm_i915_private *i915; >> + struct i915_vma *vma; >> + >> + if (!dsb) >> + return; > So the get() API returns a non-NULL pointer in both success and > failure cases, but put() can have a NULL ptr as input. Looks a bit > asymmetrical design. Also, we should add API documentation on top of > the function now, as we need to understand what can be the return > values of the function. As discussed will send NULL if user ask for DSB after reaching MAX_DSB_PER_PIPE, we can keep the null check. >> + >> + crtc = dsb->crtc; >> + i915 = to_i915(crtc->base.dev); >> + >> + if (dsb->cmd_buf) { >> + vma = dsb->vma; >> + mutex_lock(&i915->drm.struct_mutex); >> + i915_gem_object_unpin_map(vma->obj); >> + i915_vma_unpin_and_release(&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 > Same as above for license. Explained above. >> + */ >> + >> +#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 > The Bspec says there are 3 DSB DMA engines per pipe, and we have > defined MAX_DSB_PER_PIPE = 3 (and the count starts from 0), which > means we should be running our loops always < MAX_DSB_PER_PIPE. I > would prefer this to be either INVALID_DSB = 0 and DSB3 = MAX_DSB = 3, > but this is just a soft requirement, you can pick or ignore. dsb_id is used to internally to access DSB registers. >> +}; >> + >> +struct intel_dsb { >> + struct intel_crtc *crtc; >> + enum dsb_id id; > Note that your INVALID_DSB id is -1 not 0. This means all the > intel_dsb structures will be initialized with id=DSB1 not INVALID_DSB. I feel it is similar like enum used for pipe, port etc also not expecting any invalid usage. >> + u32 *cmd_buf; >> + struct i915_vma *vma; >> +}; >> + > This could also be the place to add doc about the get/put functions. Initially added per function basis and as per previous feedback created a separate patch only for docbook. >> +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 d32cfdb78b74..dd6cfb89b830 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" > > No external element is using get() / put() functions yet, this change > should go into a patch, which is using the functions. Understood your concern, the change will be little big having all dsp-api and its usage together, so splited as per different dsp-api and later a separate patch is created to enable gamma through DSB. Regards, Animesh > > - Shashank > >> #include "display/intel_frontbuffer.h" >> #include "display/intel_gmbus.h" >> #include "display/intel_opregion.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 96514dcc7812..0ab3516b0044 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1026,6 +1026,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..a2845df90573 --- /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; + + WARN_ON(i >= MAX_DSB_PER_PIPE); + + 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_DEBUG_KMS("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_DEBUG_KMS("Command buffer creation failed.\n"); + 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; + struct i915_vma *vma; + + if (!dsb) + return; + + crtc = dsb->crtc; + i915 = to_i915(crtc->base.dev); + + if (dsb->cmd_buf) { + vma = dsb->vma; + mutex_lock(&i915->drm.struct_mutex); + i915_gem_object_unpin_map(vma->obj); + i915_vma_unpin_and_release(&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 d32cfdb78b74..dd6cfb89b830 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"
The function will internally get the gem buffer from global GTT which is mapped in cpu domain to feed the data + opcode for 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) 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