Re: [PATCH v3 3/4] drm/dp: Add DisplayPort link helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jan 17, 2014 at 03:22:08PM +0200, Jani Nikula wrote:
> On Tue, 14 Jan 2014, Thierry Reding <thierry.reding@xxxxxxxxx> wrote:
[...]
> > +int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link)
> > +{
> > +	u8 values[3];
> > +	int err;
> > +
> > +	memset(link, 0, sizeof(*link));
> > +
> > +	err = drm_dp_dpcd_read(aux, DP_DPCD_REV, values, sizeof(values));
> > +	if (err < 0)
> > +		return err;
> > +
> > +	link->revision = values[0];
> > +	link->rate = drm_dp_bw_code_to_link_rate(values[1]);
> > +	link->num_lanes = values[2] & DP_MAX_LANE_COUNT_MASK;
> > +
> > +	if (values[2] & DP_ENHANCED_FRAME_CAP)
> > +		link->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
> 
> Since DP_DPCD_REV == 0, you could use the #defines for the indexes (if
> you're going to send another version anyway). Ditto below for
> drm_dp_link_configure.

We write to DP_LINK_BW_SET in drm_dp_link_configure() so I don't think
we can apply the same trick there. Also I'm not sure if it's really
worth having that here. Given that it only works if we actually read
DP_DPCD_REV, the number of locations where it can be done is fairly
small and they will look asymmetric with respect to other functions
using the drm_dp_dpcd_read/write() helpers.

So unless you feel strongly I'd prefer not to do this.

> Other than that nitpick, the series looks good to me. If we face any
> issues migrating i915 on top of this, we can iron them out later on.
> 
> On the series,
> 
> Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>

Thanks!

Thierry

Attachment: pgpEX7lsaIJv9.pgp
Description: PGP signature

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux