| Message ID | 20221005111158.3022387-1-mika.kahola@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Ensure dbuf state is defined | expand |
On Wed, Oct 05, 2022 at 02:11:58PM +0300, Mika Kahola wrote: > Ensure that the new dbuf state is not null. If so, throw > an error, discontinue to allocate ddb and return. > > Signed-off-by: Mika Kahola <mika.kahola@intel.com> > --- > drivers/gpu/drm/i915/display/skl_watermark.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c > index d58e667016e4..4ecf3b43d6d2 100644 > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > @@ -638,6 +638,9 @@ skl_crtc_allocate_ddb(struct intel_atomic_state *state, struct intel_crtc *crtc) > u32 start, end; > int ret; > > + if (IS_ERR(new_dbuf_state)) > + return PTR_ERR(new_dbuf_state); > + You can't get here without the dbuf_state. Also even if we didn't have one intel_atomic_get_new_dbuf_state() certainly shouldn't give us an error pointer (you either get a valid pointer or NULL). > if (new_dbuf_state->weight[pipe] == 0) { > skl_ddb_entry_init(&new_dbuf_state->ddb[pipe], 0, 0); > goto out; > -- > 2.34.1
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Wednesday, October 5, 2022 2:29 PM > To: Kahola, Mika <mika.kahola@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Ensure dbuf state is defined > > On Wed, Oct 05, 2022 at 02:11:58PM +0300, Mika Kahola wrote: > > Ensure that the new dbuf state is not null. If so, throw an error, > > discontinue to allocate ddb and return. > > > > Signed-off-by: Mika Kahola <mika.kahola@intel.com> > > --- > > drivers/gpu/drm/i915/display/skl_watermark.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c > > b/drivers/gpu/drm/i915/display/skl_watermark.c > > index d58e667016e4..4ecf3b43d6d2 100644 > > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > > @@ -638,6 +638,9 @@ skl_crtc_allocate_ddb(struct intel_atomic_state > *state, struct intel_crtc *crtc) > > u32 start, end; > > int ret; > > > > + if (IS_ERR(new_dbuf_state)) > > + return PTR_ERR(new_dbuf_state); > > + > > You can't get here without the dbuf_state. Also even if we didn't have one > intel_atomic_get_new_dbuf_state() certainly shouldn't give us an error pointer > (you either get a valid pointer or NULL). Ok, so we can get a null here. Should we add a check for that one? > > > if (new_dbuf_state->weight[pipe] == 0) { > > skl_ddb_entry_init(&new_dbuf_state->ddb[pipe], 0, 0); > > goto out; > > -- > > 2.34.1 > > -- > Ville Syrjälä > Intel
On Wed, Oct 05, 2022 at 12:25:26PM +0000, Kahola, Mika wrote: > > -----Original Message----- > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Sent: Wednesday, October 5, 2022 2:29 PM > > To: Kahola, Mika <mika.kahola@intel.com> > > Cc: intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Ensure dbuf state is defined > > > > On Wed, Oct 05, 2022 at 02:11:58PM +0300, Mika Kahola wrote: > > > Ensure that the new dbuf state is not null. If so, throw an error, > > > discontinue to allocate ddb and return. > > > > > > Signed-off-by: Mika Kahola <mika.kahola@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/skl_watermark.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c > > > b/drivers/gpu/drm/i915/display/skl_watermark.c > > > index d58e667016e4..4ecf3b43d6d2 100644 > > > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > > > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > > > @@ -638,6 +638,9 @@ skl_crtc_allocate_ddb(struct intel_atomic_state > > *state, struct intel_crtc *crtc) > > > u32 start, end; > > > int ret; > > > > > > + if (IS_ERR(new_dbuf_state)) > > > + return PTR_ERR(new_dbuf_state); > > > + > > > > You can't get here without the dbuf_state. Also even if we didn't have one > > intel_atomic_get_new_dbuf_state() certainly shouldn't give us an error pointer > > (you either get a valid pointer or NULL). > Ok, so we can get a null here. Should we add a check for that one? We can't get a null unless there is a bug somewhere. Is there one? > > > > > > if (new_dbuf_state->weight[pipe] == 0) { > > > skl_ddb_entry_init(&new_dbuf_state->ddb[pipe], 0, 0); > > > goto out; > > > -- > > > 2.34.1 > > > > -- > > Ville Syrjälä > > Intel
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index d58e667016e4..4ecf3b43d6d2 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -638,6 +638,9 @@ skl_crtc_allocate_ddb(struct intel_atomic_state *state, struct intel_crtc *crtc) u32 start, end; int ret; + if (IS_ERR(new_dbuf_state)) + return PTR_ERR(new_dbuf_state); + if (new_dbuf_state->weight[pipe] == 0) { skl_ddb_entry_init(&new_dbuf_state->ddb[pipe], 0, 0); goto out;
Ensure that the new dbuf state is not null. If so, throw an error, discontinue to allocate ddb and return. Signed-off-by: Mika Kahola <mika.kahola@intel.com> --- drivers/gpu/drm/i915/display/skl_watermark.c | 3 +++ 1 file changed, 3 insertions(+)