On Thu, Nov 05, 2015 at 02:10:15PM +0000, Chris Wilson wrote:
> On Wed, Nov 04, 2015 at 11:19:51PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 38 +++++++++++++-------------------------
> >  1 file changed, 13 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index a26995a..62fb95c 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1095,28 +1095,14 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct 
> > intel_connector *connector)
> >     struct drm_i915_private *dev_priv = dev->dev_private;
> >     struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >     enum port port = intel_dig_port->port;
> > -   const char *name = NULL;
> >     int ret;
> > -
> > -   switch (port) {
> > -   case PORT_A:
> > -           name = "DPDDC-A";
> > -           break;
> > -   case PORT_B:
> > -           name = "DPDDC-B";
> > -           break;
> > -   case PORT_C:
> > -           name = "DPDDC-C";
> > -           break;
> > -   case PORT_D:
> > -           name = "DPDDC-D";
> > -           break;
> > -   case PORT_E:
> > -           name = "DPDDC-E";
> > -           break;
> > -   default:
> > -           BUG();
> > -   }
> > +   static const char * const ddc_name[] = {
> > +           [PORT_A] = "DPDDC-A",
> > +           [PORT_B] = "DPDDC-B",
> > +           [PORT_C] = "DPDDC-C",
> > +           [PORT_D] = "DPDDC-D",
> > +           [PORT_E] = "DPDDC-E",
> > +   };
> >  
> >     if (INTEL_INFO(dev_priv)->gen >= 9)
> >             intel_dp->aux_ch_ctl_reg = skl_aux_ctl_reg(dev_priv, port);
> > @@ -1125,17 +1111,18 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct 
> > intel_connector *connector)
> >     else
> >             intel_dp->aux_ch_ctl_reg = g4x_aux_ctl_reg(dev_priv, port);
> >  
> > -   intel_dp->aux.name = name;
> > +   intel_dp->aux.name = ddc_name[port];
> 
> What defense do we have against the introduction of PORT_F and somebody
> forgetting to check here?

None. And I'm the one on the hook for port F actually ;)

> 
> static const char *ddc_name(enum port port) {
>       static const char * const names[] = {
>               [PORT_A] = "DPDDC-A",
>               [PORT_B] = "DPDDC-B",
>               [PORT_C] = "DPDDC-C",
>               [PORT_D] = "DPDDC-D",
>               [PORT_E] = "DPDDC-E",
>       };
>       if (WARN_ON(port >= ARRAY_SIZE(names))
>               return "DPDDC-XXX";
>       return names[port];
> }

We can do that. Or we could just do
kasprintf(GFP_KERNEL, "DPDDC-%c", port_name(port));

+ error handling and kfree() along drm_dp_aux_unregister()
of course.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to