diff mbox

[7/8] drm/i915: Assert that our hpd pin bitmasks don't overflow

Message ID 20180705164357.28512-8-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä July 5, 2018, 4:43 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Make sure our hpd pin count doesn't exceed the bitmasks we use
for tracking pending hotplugs. Not ever close to the limit yet,
but no harm in making sure either.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Chris Wilson July 5, 2018, 4:52 p.m. UTC | #1
Quoting Ville Syrjala (2018-07-05 17:43:56)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Make sure our hpd pin count doesn't exceed the bitmasks we use
> for tracking pending hotplugs. Not ever close to the limit yet,
> but no harm in making sure either.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index bb7c754979f8..c107e0837026 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1713,6 +1713,8 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
>  {
>         enum hpd_pin pin;
>  
> +       BUILD_BUG_ON(HPD_NUM_PINS > 32);

sizeof(*pin_mask) * CHAR_BIT ?

Just looking for some explanation as where the limit comes from. Now
obviously u32, but why was u32 chosen?
-Chris
Ville Syrjälä July 5, 2018, 5:51 p.m. UTC | #2
On Thu, Jul 05, 2018 at 05:52:16PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-07-05 17:43:56)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Make sure our hpd pin count doesn't exceed the bitmasks we use
> > for tracking pending hotplugs. Not ever close to the limit yet,
> > but no harm in making sure either.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index bb7c754979f8..c107e0837026 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1713,6 +1713,8 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
> >  {
> >         enum hpd_pin pin;
> >  
> > +       BUILD_BUG_ON(HPD_NUM_PINS > 32);
> 
> sizeof(*pin_mask) * CHAR_BIT ?

For a slightly more pleasing appearance I suppose we might want
to steal BITS_PER_TYPE() from include/linux/net_dim.h. A quick 
grep didn't seem to reveal anything similar in a more general
location.

> 
> Just looking for some explanation as where the limit comes from. Now
> obviously u32, but why was u32 chosen?

Had the right kind of smell perhaps?
Chris Wilson July 5, 2018, 5:57 p.m. UTC | #3
Quoting Ville Syrjälä (2018-07-05 18:51:16)
> On Thu, Jul 05, 2018 at 05:52:16PM +0100, Chris Wilson wrote:
> > Quoting Ville Syrjala (2018-07-05 17:43:56)
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Make sure our hpd pin count doesn't exceed the bitmasks we use
> > > for tracking pending hotplugs. Not ever close to the limit yet,
> > > but no harm in making sure either.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_irq.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > > index bb7c754979f8..c107e0837026 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -1713,6 +1713,8 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
> > >  {
> > >         enum hpd_pin pin;
> > >  
> > > +       BUILD_BUG_ON(HPD_NUM_PINS > 32);
> > 
> > sizeof(*pin_mask) * CHAR_BIT ?
> 
> For a slightly more pleasing appearance I suppose we might want
> to steal BITS_PER_TYPE() from include/linux/net_dim.h. A quick 
> grep didn't seem to reveal anything similar in a more general
> location.

Yup, there's quite a few places that would be made more pleasant by
BITS_PER_TYPE().

> > Just looking for some explanation as where the limit comes from. Now
> > obviously u32, but why was u32 chosen?
> 
> Had the right kind of smell perhaps?

Ok, just curious if there was an intrinsic limit that could be captured
alongside the assert.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index bb7c754979f8..c107e0837026 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1713,6 +1713,8 @@  static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
 {
 	enum hpd_pin pin;
 
+	BUILD_BUG_ON(HPD_NUM_PINS > 32);
+
 	for_each_hpd_pin(pin) {
 		if ((hpd[pin] & hotplug_trigger) == 0)
 			continue;