| Message ID | 20220324074300.21294-1-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/dsb: modified to drm_info in dsb_prepare() | expand |
On 3/24/2022 8:43 AM, Animesh Manna wrote: > The request to aqquire gem resources is failing for DSB in rare > scenario where it is busy and the register programming will be done > through mmio fallback path. > > DSB has extra advantage of faster register programming which may > go away through mmio path. Adding wait for gem resource also may > not be right as anyways losing time. > > To make the CI execution happy replaced drm_dbg_kms() to drm_info() > for printing debuf info during dsb buffer preparation. > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dsb.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c > index b34a67309976..b68dd7bd5271 100644 > --- a/drivers/gpu/drm/i915/display/intel_dsb.c > +++ b/drivers/gpu/drm/i915/display/intel_dsb.c > @@ -275,7 +275,7 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state) > > dsb = kmalloc(sizeof(*dsb), GFP_KERNEL); > if (!dsb) { > - drm_err(&i915->drm, "DSB object creation failed\n"); > + drm_info(&i915->drm, "DSB object creation failed\n"); > return; > } > > @@ -283,14 +283,14 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state) > > obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE); > if (IS_ERR(obj)) { > - drm_err(&i915->drm, "Gem object creation failed\n"); > + drm_info(&i915->drm, "Gem object creation failed\n"); If CI is happy with drm_warn then it makes sense to use drm_warn. > kfree(dsb); > goto out; > } > > vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0); > if (IS_ERR(vma)) { > - drm_err(&i915->drm, "Vma creation failed\n"); > + drm_info(&i915->drm, "Vma creation failed\n"); These messages are bit vague, add "DSB VMA creation failed" or something similar. With that Acked-by: Nirmoy Das <nirmoy.das@intel.com> Nirmoy > i915_gem_object_put(obj); > kfree(dsb); > goto out; > @@ -298,7 +298,7 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state) > > buf = i915_gem_object_pin_map_unlocked(vma->obj, I915_MAP_WC); > if (IS_ERR(buf)) { > - drm_err(&i915->drm, "Command buffer creation failed\n"); > + drm_info(&i915->drm, "Command buffer creation failed\n"); > i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP); > kfree(dsb); > goto out;
> -----Original Message----- > From: Das, Nirmoy <nirmoy.das@linux.intel.com> > Sent: Thursday, March 24, 2022 2:39 PM > To: Manna, Animesh <animesh.manna@intel.com>; intel- > gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915/dsb: modified to drm_info in > dsb_prepare() > > > On 3/24/2022 8:43 AM, Animesh Manna wrote: > > The request to aqquire gem resources is failing for DSB in rare > > scenario where it is busy and the register programming will be done > > through mmio fallback path. > > > > DSB has extra advantage of faster register programming which may go > > away through mmio path. Adding wait for gem resource also may not be > > right as anyways losing time. > > > > To make the CI execution happy replaced drm_dbg_kms() to drm_info() > > for printing debuf info during dsb buffer preparation. > > > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_dsb.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c > > b/drivers/gpu/drm/i915/display/intel_dsb.c > > index b34a67309976..b68dd7bd5271 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dsb.c > > +++ b/drivers/gpu/drm/i915/display/intel_dsb.c > > @@ -275,7 +275,7 @@ void intel_dsb_prepare(struct intel_crtc_state > > *crtc_state) > > > > dsb = kmalloc(sizeof(*dsb), GFP_KERNEL); > > if (!dsb) { > > - drm_err(&i915->drm, "DSB object creation failed\n"); > > + drm_info(&i915->drm, "DSB object creation failed\n"); > > return; > > } > > > > @@ -283,14 +283,14 @@ void intel_dsb_prepare(struct intel_crtc_state > > *crtc_state) > > > > obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE); > > if (IS_ERR(obj)) { > > - drm_err(&i915->drm, "Gem object creation failed\n"); > > + drm_info(&i915->drm, "Gem object creation failed\n"); > > If CI is happy with drm_warn then it makes sense to use drm_warn. Checked with CI team, seems drm_warn also considered as bug, is it ok to use drm_info? > > > > kfree(dsb); > > goto out; > > } > > > > vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0); > > if (IS_ERR(vma)) { > > - drm_err(&i915->drm, "Vma creation failed\n"); > > + drm_info(&i915->drm, "Vma creation failed\n"); > > > These messages are bit vague, add "DSB VMA creation failed" or something > similar. > > With that Acked-by: Nirmoy Das <nirmoy.das@intel.com> Thanks for review, will modify in next version. Regards, Animesh > > > Nirmoy > > > > i915_gem_object_put(obj); > > kfree(dsb); > > goto out; > > @@ -298,7 +298,7 @@ void intel_dsb_prepare(struct intel_crtc_state > > *crtc_state) > > > > buf = i915_gem_object_pin_map_unlocked(vma->obj, I915_MAP_WC); > > if (IS_ERR(buf)) { > > - drm_err(&i915->drm, "Command buffer creation failed\n"); > > + drm_info(&i915->drm, "Command buffer creation failed\n"); > > i915_vma_unpin_and_release(&vma, > I915_VMA_RELEASE_MAP); > > kfree(dsb); > > goto out;
On 3/25/2022 2:22 PM, Manna, Animesh wrote: > >> -----Original Message----- >> From: Das, Nirmoy <nirmoy.das@linux.intel.com> >> Sent: Thursday, March 24, 2022 2:39 PM >> To: Manna, Animesh <animesh.manna@intel.com>; intel- >> gfx@lists.freedesktop.org >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/dsb: modified to drm_info in >> dsb_prepare() >> >> >> On 3/24/2022 8:43 AM, Animesh Manna wrote: >>> The request to aqquire gem resources is failing for DSB in rare >>> scenario where it is busy and the register programming will be done >>> through mmio fallback path. >>> >>> DSB has extra advantage of faster register programming which may go >>> away through mmio path. Adding wait for gem resource also may not be >>> right as anyways losing time. >>> >>> To make the CI execution happy replaced drm_dbg_kms() to drm_info() >>> for printing debuf info during dsb buffer preparation. >>> >>> Signed-off-by: Animesh Manna <animesh.manna@intel.com> >>> --- >>> drivers/gpu/drm/i915/display/intel_dsb.c | 8 ++++---- >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c >>> b/drivers/gpu/drm/i915/display/intel_dsb.c >>> index b34a67309976..b68dd7bd5271 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_dsb.c >>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c >>> @@ -275,7 +275,7 @@ void intel_dsb_prepare(struct intel_crtc_state >>> *crtc_state) >>> >>> dsb = kmalloc(sizeof(*dsb), GFP_KERNEL); >>> if (!dsb) { >>> - drm_err(&i915->drm, "DSB object creation failed\n"); >>> + drm_info(&i915->drm, "DSB object creation failed\n"); >>> return; >>> } >>> >>> @@ -283,14 +283,14 @@ void intel_dsb_prepare(struct intel_crtc_state >>> *crtc_state) >>> >>> obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE); >>> if (IS_ERR(obj)) { >>> - drm_err(&i915->drm, "Gem object creation failed\n"); >>> + drm_info(&i915->drm, "Gem object creation failed\n"); >> If CI is happy with drm_warn then it makes sense to use drm_warn. > Checked with CI team, seems drm_warn also considered as bug, is it ok to use drm_info? In that case: don't print anything on each error and at out label, you can print an info about the fallback option. Nirmoy > >> >>> kfree(dsb); >>> goto out; >>> } >>> >>> vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0); >>> if (IS_ERR(vma)) { >>> - drm_err(&i915->drm, "Vma creation failed\n"); >>> + drm_info(&i915->drm, "Vma creation failed\n"); >> >> These messages are bit vague, add "DSB VMA creation failed" or something >> similar. >> >> With that Acked-by: Nirmoy Das <nirmoy.das@intel.com> > Thanks for review, will modify in next version. > > Regards, > Animesh > >> >> Nirmoy >> >> >>> i915_gem_object_put(obj); >>> kfree(dsb); >>> goto out; >>> @@ -298,7 +298,7 @@ void intel_dsb_prepare(struct intel_crtc_state >>> *crtc_state) >>> >>> buf = i915_gem_object_pin_map_unlocked(vma->obj, I915_MAP_WC); >>> if (IS_ERR(buf)) { >>> - drm_err(&i915->drm, "Command buffer creation failed\n"); >>> + drm_info(&i915->drm, "Command buffer creation failed\n"); >>> i915_vma_unpin_and_release(&vma, >> I915_VMA_RELEASE_MAP); >>> kfree(dsb); >>> goto out;
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c index b34a67309976..b68dd7bd5271 100644 --- a/drivers/gpu/drm/i915/display/intel_dsb.c +++ b/drivers/gpu/drm/i915/display/intel_dsb.c @@ -275,7 +275,7 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state) dsb = kmalloc(sizeof(*dsb), GFP_KERNEL); if (!dsb) { - drm_err(&i915->drm, "DSB object creation failed\n"); + drm_info(&i915->drm, "DSB object creation failed\n"); return; } @@ -283,14 +283,14 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state) obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE); if (IS_ERR(obj)) { - drm_err(&i915->drm, "Gem object creation failed\n"); + drm_info(&i915->drm, "Gem object creation failed\n"); kfree(dsb); goto out; } vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0); if (IS_ERR(vma)) { - drm_err(&i915->drm, "Vma creation failed\n"); + drm_info(&i915->drm, "Vma creation failed\n"); i915_gem_object_put(obj); kfree(dsb); goto out; @@ -298,7 +298,7 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state) buf = i915_gem_object_pin_map_unlocked(vma->obj, I915_MAP_WC); if (IS_ERR(buf)) { - drm_err(&i915->drm, "Command buffer creation failed\n"); + drm_info(&i915->drm, "Command buffer creation failed\n"); i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP); kfree(dsb); goto out;
The request to aqquire gem resources is failing for DSB in rare scenario where it is busy and the register programming will be done through mmio fallback path. DSB has extra advantage of faster register programming which may go away through mmio path. Adding wait for gem resource also may not be right as anyways losing time. To make the CI execution happy replaced drm_dbg_kms() to drm_info() for printing debuf info during dsb buffer preparation. Signed-off-by: Animesh Manna <animesh.manna@intel.com> --- drivers/gpu/drm/i915/display/intel_dsb.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)