| Message ID | 1432895827-5185-2-git-send-email-gaurav.k.singh@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, May 29, 2015 at 04:06:53PM +0530, Gaurav K Singh wrote: > Allocate gem memory for MIPI DBI command buffer. This memory > will be used when sending command via DBI interface. Why would you allocate this via gem? AFAICS you only feed the bus address to the hardware. Using the dma-api would seem like the right choice here, but I'm not sure how to deal with the dma mask. > > Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com> > Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com> > Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> > --- > drivers/gpu/drm/i915/intel_dsi.c | 31 +++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_dsi.h | 4 ++++ > 2 files changed, 35 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index 5196642..2e3c801 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -415,6 +415,27 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder) > > DRM_DEBUG_KMS("\n"); > > + if (!intel_dsi->gem_obj && is_cmd_mode(intel_dsi)) { > + intel_dsi->gem_obj = i915_gem_alloc_object(dev, 4096); > + if (!intel_dsi->gem_obj) { > + DRM_ERROR("Failed to allocate seqno page\n"); > + return; > + } > + > + i915_gem_object_set_cache_level(intel_dsi->gem_obj, > + I915_CACHE_LLC); > + > + if (i915_gem_obj_ggtt_pin(intel_dsi->gem_obj, 4096, 0)) { > + DRM_ERROR("MIPI command buffer GTT pin failed"); > + return; > + } > + > + intel_dsi->cmd_buff = > + kmap(sg_page(intel_dsi->gem_obj->pages->sgl)); > + intel_dsi->cmd_buff_phy_addr = page_to_phys( > + sg_page(intel_dsi->gem_obj->pages->sgl)); > + } > + > /* Disable DPOunit clock gating, can stall pipe > * and we need DPLL REFA always enabled */ > tmp = I915_READ(DPLL(pipe)); > @@ -576,6 +597,12 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder) > > msleep(intel_dsi->panel_off_delay); > msleep(intel_dsi->panel_pwr_cycle_delay); > + > + if (intel_dsi->gem_obj) { > + kunmap(intel_dsi->cmd_buff); > + i915_gem_object_ggtt_unpin(intel_dsi->gem_obj); > + drm_gem_object_unreference(&intel_dsi->gem_obj->base); > + } > } > > static bool intel_dsi_get_hw_state(struct intel_encoder *encoder, > @@ -1048,6 +1075,10 @@ void intel_dsi_init(struct drm_device *dev) > intel_dsi->ports = (1 << PORT_C); > } > > + intel_dsi->cmd_buff = NULL; > + intel_dsi->cmd_buff_phy_addr = 0; > + intel_dsi->gem_obj = NULL; > + > /* Create a DSI host (and a device) for each port. */ > for_each_dsi_port(port, intel_dsi->ports) { > struct intel_dsi_host *host; > diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h > index 2784ac4..36ca3cc 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.h > +++ b/drivers/gpu/drm/i915/intel_dsi.h > @@ -44,6 +44,10 @@ struct intel_dsi { > > struct intel_connector *attached_connector; > > + struct drm_i915_gem_object *gem_obj; > + void *cmd_buff; > + dma_addr_t cmd_buff_phy_addr; > + > /* bit mask of ports being driven */ > u16 ports; > > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, May 29, 2015 at 01:59:01PM +0300, Ville Syrjälä wrote: > On Fri, May 29, 2015 at 04:06:53PM +0530, Gaurav K Singh wrote: > > Allocate gem memory for MIPI DBI command buffer. This memory > > will be used when sending command via DBI interface. > > Why would you allocate this via gem? AFAICS you only feed the bus > address to the hardware. Using the dma-api would seem like the right > choice here, but I'm not sure how to deal with the dma mask. Yeah dma_alloc_coherent is what you want here. The mask can be ignored, it should be suitable already. -Daniel > > > > > Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com> > > Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com> > > Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> > > --- > > drivers/gpu/drm/i915/intel_dsi.c | 31 +++++++++++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_dsi.h | 4 ++++ > > 2 files changed, 35 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > > index 5196642..2e3c801 100644 > > --- a/drivers/gpu/drm/i915/intel_dsi.c > > +++ b/drivers/gpu/drm/i915/intel_dsi.c > > @@ -415,6 +415,27 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder) > > > > DRM_DEBUG_KMS("\n"); > > > > + if (!intel_dsi->gem_obj && is_cmd_mode(intel_dsi)) { > > + intel_dsi->gem_obj = i915_gem_alloc_object(dev, 4096); > > + if (!intel_dsi->gem_obj) { > > + DRM_ERROR("Failed to allocate seqno page\n"); > > + return; > > + } > > + > > + i915_gem_object_set_cache_level(intel_dsi->gem_obj, > > + I915_CACHE_LLC); > > + > > + if (i915_gem_obj_ggtt_pin(intel_dsi->gem_obj, 4096, 0)) { > > + DRM_ERROR("MIPI command buffer GTT pin failed"); > > + return; > > + } > > + > > + intel_dsi->cmd_buff = > > + kmap(sg_page(intel_dsi->gem_obj->pages->sgl)); > > + intel_dsi->cmd_buff_phy_addr = page_to_phys( > > + sg_page(intel_dsi->gem_obj->pages->sgl)); > > + } > > + > > /* Disable DPOunit clock gating, can stall pipe > > * and we need DPLL REFA always enabled */ > > tmp = I915_READ(DPLL(pipe)); > > @@ -576,6 +597,12 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder) > > > > msleep(intel_dsi->panel_off_delay); > > msleep(intel_dsi->panel_pwr_cycle_delay); > > + > > + if (intel_dsi->gem_obj) { > > + kunmap(intel_dsi->cmd_buff); > > + i915_gem_object_ggtt_unpin(intel_dsi->gem_obj); > > + drm_gem_object_unreference(&intel_dsi->gem_obj->base); > > + } > > } > > > > static bool intel_dsi_get_hw_state(struct intel_encoder *encoder, > > @@ -1048,6 +1075,10 @@ void intel_dsi_init(struct drm_device *dev) > > intel_dsi->ports = (1 << PORT_C); > > } > > > > + intel_dsi->cmd_buff = NULL; > > + intel_dsi->cmd_buff_phy_addr = 0; > > + intel_dsi->gem_obj = NULL; > > + > > /* Create a DSI host (and a device) for each port. */ > > for_each_dsi_port(port, intel_dsi->ports) { > > struct intel_dsi_host *host; > > diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h > > index 2784ac4..36ca3cc 100644 > > --- a/drivers/gpu/drm/i915/intel_dsi.h > > +++ b/drivers/gpu/drm/i915/intel_dsi.h > > @@ -44,6 +44,10 @@ struct intel_dsi { > > > > struct intel_connector *attached_connector; > > > > + struct drm_i915_gem_object *gem_obj; > > + void *cmd_buff; > > + dma_addr_t cmd_buff_phy_addr; > > + > > /* bit mask of ports being driven */ > > u16 ports; > > > > -- > > 1.7.9.5 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, May 29, 2015 at 07:10:53PM +0200, Daniel Vetter wrote: > On Fri, May 29, 2015 at 01:59:01PM +0300, Ville Syrjälä wrote: > > On Fri, May 29, 2015 at 04:06:53PM +0530, Gaurav K Singh wrote: > > > Allocate gem memory for MIPI DBI command buffer. This memory > > > will be used when sending command via DBI interface. > > > > Why would you allocate this via gem? AFAICS you only feed the bus > > address to the hardware. Using the dma-api would seem like the right > > choice here, but I'm not sure how to deal with the dma mask. > > Yeah dma_alloc_coherent is what you want here. The mask can be ignored, > it should be suitable already. Umm, this thing seems to limited to 32bit addresses. And we set the mask to 39 or 40 bits depending on the gen.
On Mon, Jun 01, 2015 at 02:03:15PM +0300, Ville Syrjälä wrote: > On Fri, May 29, 2015 at 07:10:53PM +0200, Daniel Vetter wrote: > > On Fri, May 29, 2015 at 01:59:01PM +0300, Ville Syrjälä wrote: > > > On Fri, May 29, 2015 at 04:06:53PM +0530, Gaurav K Singh wrote: > > > > Allocate gem memory for MIPI DBI command buffer. This memory > > > > will be used when sending command via DBI interface. > > > > > > Why would you allocate this via gem? AFAICS you only feed the bus > > > address to the hardware. Using the dma-api would seem like the right > > > choice here, but I'm not sure how to deal with the dma mask. > > > > Yeah dma_alloc_coherent is what you want here. The mask can be ignored, > > it should be suitable already. > > Umm, this thing seems to limited to 32bit addresses. And we set the mask > to 39 or 40 bits depending on the gen. Well that's what we have dma_set_coherent_mask for, hooray. Not the first one, see the other hacks with comments in i915_driver_load. -Daniel
On 6/15/2015 4:00 PM, Daniel Vetter wrote: > On Mon, Jun 01, 2015 at 02:03:15PM +0300, Ville Syrjälä wrote: >> On Fri, May 29, 2015 at 07:10:53PM +0200, Daniel Vetter wrote: >>> On Fri, May 29, 2015 at 01:59:01PM +0300, Ville Syrjälä wrote: >>>> On Fri, May 29, 2015 at 04:06:53PM +0530, Gaurav K Singh wrote: >>>>> Allocate gem memory for MIPI DBI command buffer. This memory >>>>> will be used when sending command via DBI interface. >>>> Why would you allocate this via gem? AFAICS you only feed the bus >>>> address to the hardware. Using the dma-api would seem like the right >>>> choice here, but I'm not sure how to deal with the dma mask. >>> Yeah dma_alloc_coherent is what you want here. The mask can be ignored, >>> it should be suitable already. >> Umm, this thing seems to limited to 32bit addresses. And we set the mask >> to 39 or 40 bits depending on the gen. > Well that's what we have dma_set_coherent_mask for, hooray. Not the first > one, see the other hacks with comments in i915_driver_load. > -Daniel checking on the usage of dma_alloc_coherent. Will come back on this. With regards, Gaurav
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index 5196642..2e3c801 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -415,6 +415,27 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder) DRM_DEBUG_KMS("\n"); + if (!intel_dsi->gem_obj && is_cmd_mode(intel_dsi)) { + intel_dsi->gem_obj = i915_gem_alloc_object(dev, 4096); + if (!intel_dsi->gem_obj) { + DRM_ERROR("Failed to allocate seqno page\n"); + return; + } + + i915_gem_object_set_cache_level(intel_dsi->gem_obj, + I915_CACHE_LLC); + + if (i915_gem_obj_ggtt_pin(intel_dsi->gem_obj, 4096, 0)) { + DRM_ERROR("MIPI command buffer GTT pin failed"); + return; + } + + intel_dsi->cmd_buff = + kmap(sg_page(intel_dsi->gem_obj->pages->sgl)); + intel_dsi->cmd_buff_phy_addr = page_to_phys( + sg_page(intel_dsi->gem_obj->pages->sgl)); + } + /* Disable DPOunit clock gating, can stall pipe * and we need DPLL REFA always enabled */ tmp = I915_READ(DPLL(pipe)); @@ -576,6 +597,12 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder) msleep(intel_dsi->panel_off_delay); msleep(intel_dsi->panel_pwr_cycle_delay); + + if (intel_dsi->gem_obj) { + kunmap(intel_dsi->cmd_buff); + i915_gem_object_ggtt_unpin(intel_dsi->gem_obj); + drm_gem_object_unreference(&intel_dsi->gem_obj->base); + } } static bool intel_dsi_get_hw_state(struct intel_encoder *encoder, @@ -1048,6 +1075,10 @@ void intel_dsi_init(struct drm_device *dev) intel_dsi->ports = (1 << PORT_C); } + intel_dsi->cmd_buff = NULL; + intel_dsi->cmd_buff_phy_addr = 0; + intel_dsi->gem_obj = NULL; + /* Create a DSI host (and a device) for each port. */ for_each_dsi_port(port, intel_dsi->ports) { struct intel_dsi_host *host; diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h index 2784ac4..36ca3cc 100644 --- a/drivers/gpu/drm/i915/intel_dsi.h +++ b/drivers/gpu/drm/i915/intel_dsi.h @@ -44,6 +44,10 @@ struct intel_dsi { struct intel_connector *attached_connector; + struct drm_i915_gem_object *gem_obj; + void *cmd_buff; + dma_addr_t cmd_buff_phy_addr; + /* bit mask of ports being driven */ u16 ports;