Re: [alsa-devel] [PATCH 12/15] drm/edid: Add API to help find connection type

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

 



On Thu, 03 Dec 2015, "Subhransu S. Prusty" <subhransu.s.prusty@xxxxxxxxx> wrote:
> On Wed, Dec 02, 2015 at 06:07:01PM +0100, Thierry Reding wrote:
>> On Wed, Dec 02, 2015 at 11:53:02AM +0200, Jani Nikula wrote:
>> > On Tue, 01 Dec 2015, "Subhransu S. Prusty" <subhransu.s.prusty@xxxxxxxxx> wrote:
>> > > To fill the audio infoframe it is required to identify the connection type
>> > > as DP or HDMI. So parse the required bits in ELD to find the connection
>> > > type.
>> > >
>> > > Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@xxxxxxxxx>
>> > > Signed-off-by: Vinod Koul <vinod.koul@xxxxxxxxx>
>> > > Cc: David Airlie <airlied@xxxxxxxx>
>> > > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
>> > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx>
>> > > ---
>> > >  include/drm/drm_edid.h | 10 ++++++++++
>> > >  1 file changed, 10 insertions(+)
>> > >
>> > > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
>> > > index 2af9769..c7595a5 100644
>> > > --- a/include/drm/drm_edid.h
>> > > +++ b/include/drm/drm_edid.h
>> > > @@ -403,6 +403,16 @@ static inline int drm_eld_size(const uint8_t *eld)
>> > >  	return DRM_ELD_HEADER_BLOCK_SIZE + eld[DRM_ELD_BASELINE_ELD_LEN] * 4;
>> > >  }
>> > >  
>> > > +/**
>> > > + * drm_eld_get_conn_type - Get device type hdmi/dp connected
>> > > + * @eld: pointer to an eld memory structure
>> > > + */
>> > > +static inline int drm_eld_get_conn_type(const uint8_t *eld)
>> > > +{
>> > > +	return (eld[DRM_ELD_SAD_COUNT_CONN_TYPE] & DRM_ELD_CONN_TYPE_MASK) >>
>> > > +		DRM_ELD_CONN_TYPE_SHIFT;
>> > > +}
>> > 
>> > I'm not sure how much this helps when the caller still needs to
>> > magically know what the return value means...  Indeed the next patch
>> > with /* 0 is hdmi and 1 is DP */ and "conn_type == 0" is a bit ugly.
>> > 
>> > How about just not shifting the return value, and using
>> > DRM_ELD_CONN_TYPE_HDMI and DRM_ELD_CONN_TYPE_DP in the caller? Bonus
>> > points for referencing those in the kernel-doc above.
>> 
>> We already have a similar function for detecting HDMI vs. DVI (see the
>> drm_detect_hdmi_monitor()), so perhaps adhering to that convention might
>> be preferable. This could be:
>> 
>> 	static inline bool drm_eld_detect_dp(const u8 *eld)
>> 	{
>> 		u8 type = eld[DRM_ELD_SAD_COUNT_CONN_TYPE] & DRM_ELD_CONN_TYPE_MASK;
>> 
>> 		return type == DRM_ELD_CONN_TYPE_DP;
>> 	}
>
> With this approach it needs two APIs to be added for HDMI or DP
> detection. So I prefer what Jani suggested and caller compares
> whether it is HDMI/DP connection type. Will updae the kernel doc
> for the same as well.

I presume Thierry means you'd assume HDMI if drm_eld_detect_dp() returns
false.

I'm fine with either approach.

BR,
Jani.

>
>> 
>> Thierry
>
>
>
>> _______________________________________________
>> Alsa-devel mailing list
>> Alsa-devel@xxxxxxxxxxxxxxxx
>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
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