> -----Original Message-----
> From: Manna, Animesh
> Sent: Wednesday, January 4, 2023 12:28 PM
> To: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>; intel-
> gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: RE: [PATCH 04/13] drm/i915/dsb: Fix DSB command
> buffer size checks
>
> Hi,
>
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
> > Ville Syrjala
> > Sent: Friday, December 16, 2022 6:08 AM
> > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > Subject: [PATCH 04/13] drm/i915/dsb: Fix DSB command
> > buffer size checks
> >
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > free_pos is in dwords, DSB_BUF_SIZE in bytes. Directly comparing the
> > two is nonsense. Fix it up, and make sure we also account for the
> > 8byte alignment requirement for each instruction, and also assume that
> > each instruction normally eats two dwords.
>
> free_pos variable is used as index to fill the dsb command buffer and getting
> filed byte by byte. It is not in dwords.
> Below given the structure definition of dsb cmd_buf.
> struct intel_dsb {
> enum dsb_id id;
>
> u32 *cmd_buf;
Please ignore my previous comments ...
Regards,
Animesh
>
>
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/intel_dsb.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c
> > b/drivers/gpu/drm/i915/display/intel_dsb.c
> > index 6abfd0fc541a..fbcbf9efd039 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> > @@ -97,7 +97,7 @@ void intel_dsb_indexed_reg_write(struct intel_dsb
> > *dsb,
> > u32 *buf = dsb->cmd_buf;
> > u32 reg_val;
> >
> > - if (drm_WARN_ON(&dev_priv->drm, dsb->free_pos >=
> > DSB_BUF_SIZE)) {
> > + if (drm_WARN_ON(&dev_priv->drm, ALIGN(dsb->free_pos, 2) >
> > DSB_BUF_SIZE
> > +/ 4 - 2)) {
>
> Here DSB_BUF_SIZE is 4096 x 2 = 8192 bytes DSB_BUF_SIZE / 4 - 2 = 2046
> bytes.
> With the above logic warning will be triggered after 2046 bytes.
>
> Regards,
> Animesh
>
>
> > drm_dbg_kms(&dev_priv->drm, "DSB buffer overflow\n");
> > return;
> > }
> > @@ -167,7 +167,7 @@ void intel_dsb_reg_write(struct intel_dsb *dsb,
> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > u32 *buf = dsb->cmd_buf;
> >
> > - if (drm_WARN_ON(&dev_priv->drm, dsb->free_pos >=
> > DSB_BUF_SIZE)) {
> > + if (drm_WARN_ON(&dev_priv->drm, ALIGN(dsb->free_pos, 2) >
> > DSB_BUF_SIZE
> > +/ 4 - 2)) {
> > drm_dbg_kms(&dev_priv->drm, "DSB buffer overflow\n");
> > return;
> > }
> > --
> > 2.37.4
