Re: [RFC PATCH 7/7] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

2017-05-31 Thread Hans Verkuil

On 05/31/2017 01:25 AM, Clint Taylor wrote:



On 05/30/2017 02:29 PM, Hans Verkuil wrote:

On 05/30/2017 10:32 PM, Clint Taylor wrote:



On 05/30/2017 09:54 AM, Hans Verkuil wrote:

On 05/30/2017 06:49 PM, Hans Verkuil wrote:

On 05/30/2017 04:19 PM, Clint Taylor wrote:



On 05/30/2017 12:11 AM, Jani Nikula wrote:

On Tue, 30 May 2017, Hans Verkuil  wrote:

On 05/29/2017 09:00 PM, Daniel Vetter wrote:

On Fri, May 26, 2017 at 12:20:48PM +0200, Hans Verkuil wrote:

On 05/26/2017 09:15 AM, Daniel Vetter wrote:

Did you look into also wiring this up for dp mst chains?

Isn't this sufficient? I have no way of testing mst chains.

I think I need some pointers from you, since I am a complete
newbie when it
comes to mst.

I don't really have more clue, but yeah if you don't have an mst
thing (a
simple dp port multiplexer is what I use for testing here, then
plug in a
converter dongle or cable into that) then probably better to not
wire up
the code for it.

I think my NUC already uses mst internally. But I was planning on
buying a
dp multiplexer to make sure there is nothing special I need to do
for mst.

The CEC Tunneling is all in the branch device, so if I understand
things
correctly it is not affected by mst.

BTW, I did a bit more testing on my NUC7i5BNK: for the HDMI output
they
use a MegaChip MCDP2800 DP-to-HDMI converter which according to
their
datasheet is supposed to implement CEC Tunneling, but if they do
it is not
exposed as a capability. I'm not sure if it is a MegaChip firmware
issue
or something else. The BIOS is able to do some CEC, but whether
they hook
into the MegaChip or have the CEC pin connected to a GPIO or
something and
have their own controller is something I do not know.

If anyone can clarify what Intel did on the NUC, then that would
be very
helpful.

It's called LSPCON, see i915/intel_lspcon.c, basically to support
HDMI
2.0. Currently we only use it in PCON mode, shows up as DP for
us. It
could be used in LS mode, showing up as HDMI 1.4, but we don't
support
that in i915.

I don't know about the CEC on that.


My NUC6i7KYK has the MCDP2850 LSPCON and it does support CEC over
Aux.
The release notes for the NUC state that there is a BIOS
configuration
option for enabling support. My doesn't have the option but the
LSPCON
fully supports CEC.


What is the output of:

dd if=/dev/drm_dp_aux0 of=aux0 skip=12288 ibs=1 count=48
od -t x1 aux0

Assuming drm_dp_aux0 is the aux channel for the HDMI output on your
NUC.

If the first byte is != 0x00, then it advertises CEC over Aux.

For me it says 0x00.

When you say "it does support CEC over Aux", does that mean you have
actually
tested it somehow? The only working solution I have seen mentioned
for the
NUC6i7KYK is a Pulse-Eight adapter.

With the NUC7i Intel made BIOS support for CEC, but it is not at all
clear to me if they used CEC tunneling or just hooked up the CEC
pin to
some microcontroller.

The only working chipset I have seen is the Parade PS176.


If it really is working on your NUC, then can you add the output of
/sys/kernel/debug/dri/0/i915_display_info?


[root@localhost cec-ctl]# cat /sys/kernel/debug/dri/0/i915_display_info




Connector info
--
connector 48: type DP-1, status: connected
   name:
   physical dimensions: 700x400mm
   subpixel order: Unknown
   CEA rev: 3
   DPCD rev: 12
   audio support: yes
   DP branch device present: yes
   Type: HDMI
   ID: 175IB0
   HW: 1.0
   SW: 7.32
   Max TMDS clock: 60 kHz
   Max bpc: 12


Huh. Based on this document:

https://downloadmirror.intel.com/26061/eng/NUC6i7KYK%20HDMI%202.0%20Firmware%20update%20Instructions.pdf


this is the internal DP-to-HDMI adapter and it has the PS175. So it is a
Parade chipset, and I have seen that work before (at least the PS176).

This is the PS175 LSPCON on the NUC6.





connector 55: type DP-2, status: connected
   name:
   physical dimensions: 620x340mm
   subpixel order: Unknown
   CEA rev: 3
   DPCD rev: 12
   audio support: yes
   DP branch device present: yes
   Type: HDMI
   ID: BCTRC0
   HW: 2.0
   SW: 0.26


And is this from a USB-C to HDMI adapter? Which one? I don't recognize
the ID.


This is a LSPCON inside the Google USB-C->HDMI dongle. This is actually
a MC2850 with what appears to be a custom ID. Datasheet claims CEC over
Aux and the pin is wired, but FW has it currently disabled.


OK, in other words the Parade chipsets work and the Megachip chipsets
don't. And Intel in their wisdom decided to go with Megachip for the new
NUCs.

I have no idea if you have any ins with the NUC team, but it would be so
nice if there would be a Megachip firmware update enabling this feature.

Regards,

Hans


Re: [RFC PATCH 7/7] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

2017-05-30 Thread Clint Taylor



On 05/30/2017 02:29 PM, Hans Verkuil wrote:

On 05/30/2017 10:32 PM, Clint Taylor wrote:



On 05/30/2017 09:54 AM, Hans Verkuil wrote:

On 05/30/2017 06:49 PM, Hans Verkuil wrote:

On 05/30/2017 04:19 PM, Clint Taylor wrote:



On 05/30/2017 12:11 AM, Jani Nikula wrote:

On Tue, 30 May 2017, Hans Verkuil  wrote:

On 05/29/2017 09:00 PM, Daniel Vetter wrote:

On Fri, May 26, 2017 at 12:20:48PM +0200, Hans Verkuil wrote:

On 05/26/2017 09:15 AM, Daniel Vetter wrote:

Did you look into also wiring this up for dp mst chains?

Isn't this sufficient? I have no way of testing mst chains.

I think I need some pointers from you, since I am a complete
newbie when it
comes to mst.

I don't really have more clue, but yeah if you don't have an mst
thing (a
simple dp port multiplexer is what I use for testing here, then
plug in a
converter dongle or cable into that) then probably better to not
wire up
the code for it.

I think my NUC already uses mst internally. But I was planning on
buying a
dp multiplexer to make sure there is nothing special I need to do
for mst.

The CEC Tunneling is all in the branch device, so if I understand
things
correctly it is not affected by mst.

BTW, I did a bit more testing on my NUC7i5BNK: for the HDMI output
they
use a MegaChip MCDP2800 DP-to-HDMI converter which according to 
their

datasheet is supposed to implement CEC Tunneling, but if they do
it is not
exposed as a capability. I'm not sure if it is a MegaChip firmware
issue
or something else. The BIOS is able to do some CEC, but whether
they hook
into the MegaChip or have the CEC pin connected to a GPIO or
something and
have their own controller is something I do not know.

If anyone can clarify what Intel did on the NUC, then that would
be very
helpful.
It's called LSPCON, see i915/intel_lspcon.c, basically to support 
HDMI
2.0. Currently we only use it in PCON mode, shows up as DP for 
us. It
could be used in LS mode, showing up as HDMI 1.4, but we don't 
support

that in i915.

I don't know about the CEC on that.


My NUC6i7KYK has the MCDP2850 LSPCON and it does support CEC over 
Aux.
The release notes for the NUC state that there is a BIOS 
configuration
option for enabling support. My doesn't have the option but the 
LSPCON

fully supports CEC.


What is the output of:

dd if=/dev/drm_dp_aux0 of=aux0 skip=12288 ibs=1 count=48
od -t x1 aux0

Assuming drm_dp_aux0 is the aux channel for the HDMI output on your 
NUC.


If the first byte is != 0x00, then it advertises CEC over Aux.

For me it says 0x00.

When you say "it does support CEC over Aux", does that mean you have
actually
tested it somehow? The only working solution I have seen mentioned
for the
NUC6i7KYK is a Pulse-Eight adapter.

With the NUC7i Intel made BIOS support for CEC, but it is not at all
clear to me if they used CEC tunneling or just hooked up the CEC 
pin to

some microcontroller.

The only working chipset I have seen is the Parade PS176.


If it really is working on your NUC, then can you add the output of
/sys/kernel/debug/dri/0/i915_display_info?


[root@localhost cec-ctl]# cat /sys/kernel/debug/dri/0/i915_display_info




Connector info
--
connector 48: type DP-1, status: connected
  name:
  physical dimensions: 700x400mm
  subpixel order: Unknown
  CEA rev: 3
  DPCD rev: 12
  audio support: yes
  DP branch device present: yes
  Type: HDMI
  ID: 175IB0
  HW: 1.0
  SW: 7.32
  Max TMDS clock: 60 kHz
  Max bpc: 12


Huh. Based on this document:

https://downloadmirror.intel.com/26061/eng/NUC6i7KYK%20HDMI%202.0%20Firmware%20update%20Instructions.pdf 



this is the internal DP-to-HDMI adapter and it has the PS175. So it is a
Parade chipset, and I have seen that work before (at least the PS176).

This is the PS175 LSPCON on the NUC6.





connector 55: type DP-2, status: connected
  name:
  physical dimensions: 620x340mm
  subpixel order: Unknown
  CEA rev: 3
  DPCD rev: 12
  audio support: yes
  DP branch device present: yes
  Type: HDMI
  ID: BCTRC0
  HW: 2.0
  SW: 0.26


And is this from a USB-C to HDMI adapter? Which one? I don't recognize 
the ID.


This is a LSPCON inside the Google USB-C->HDMI dongle. This is actually 
a MC2850 with what appears to be a custom ID. Datasheet claims CEC over 
Aux and the pin is wired, but FW has it currently disabled.


-Clint


For the record, this is the internal HDMI output of my NUC7i5BNK:

connector 48: type DP-1, status: connected
name:
physical dimensions: 1050x590mm
subpixel order: Unknown
CEA rev: 3
DPCD rev: 12
audio support: yes
DP branch device present: yes
Type: HDMI
ID: MC2800
HW: 2.2
SW: 1.66
Max TMDS clock: 60 kHz
Max bpc: 16

Clearly a Megachip.

Regards,

Hans




Re: [RFC PATCH 7/7] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

2017-05-30 Thread Hans Verkuil

On 05/30/2017 10:32 PM, Clint Taylor wrote:



On 05/30/2017 09:54 AM, Hans Verkuil wrote:

On 05/30/2017 06:49 PM, Hans Verkuil wrote:

On 05/30/2017 04:19 PM, Clint Taylor wrote:



On 05/30/2017 12:11 AM, Jani Nikula wrote:

On Tue, 30 May 2017, Hans Verkuil  wrote:

On 05/29/2017 09:00 PM, Daniel Vetter wrote:

On Fri, May 26, 2017 at 12:20:48PM +0200, Hans Verkuil wrote:

On 05/26/2017 09:15 AM, Daniel Vetter wrote:

Did you look into also wiring this up for dp mst chains?

Isn't this sufficient? I have no way of testing mst chains.

I think I need some pointers from you, since I am a complete
newbie when it
comes to mst.

I don't really have more clue, but yeah if you don't have an mst
thing (a
simple dp port multiplexer is what I use for testing here, then
plug in a
converter dongle or cable into that) then probably better to not
wire up
the code for it.

I think my NUC already uses mst internally. But I was planning on
buying a
dp multiplexer to make sure there is nothing special I need to do
for mst.

The CEC Tunneling is all in the branch device, so if I understand
things
correctly it is not affected by mst.

BTW, I did a bit more testing on my NUC7i5BNK: for the HDMI output
they
use a MegaChip MCDP2800 DP-to-HDMI converter which according to their
datasheet is supposed to implement CEC Tunneling, but if they do
it is not
exposed as a capability. I'm not sure if it is a MegaChip firmware
issue
or something else. The BIOS is able to do some CEC, but whether
they hook
into the MegaChip or have the CEC pin connected to a GPIO or
something and
have their own controller is something I do not know.

If anyone can clarify what Intel did on the NUC, then that would
be very
helpful.

It's called LSPCON, see i915/intel_lspcon.c, basically to support HDMI
2.0. Currently we only use it in PCON mode, shows up as DP for us. It
could be used in LS mode, showing up as HDMI 1.4, but we don't support
that in i915.

I don't know about the CEC on that.


My NUC6i7KYK has the MCDP2850 LSPCON and it does support CEC over Aux.
The release notes for the NUC state that there is a BIOS configuration
option for enabling support. My doesn't have the option but the LSPCON
fully supports CEC.


What is the output of:

dd if=/dev/drm_dp_aux0 of=aux0 skip=12288 ibs=1 count=48
od -t x1 aux0

Assuming drm_dp_aux0 is the aux channel for the HDMI output on your NUC.

If the first byte is != 0x00, then it advertises CEC over Aux.

For me it says 0x00.

When you say "it does support CEC over Aux", does that mean you have
actually
tested it somehow? The only working solution I have seen mentioned
for the
NUC6i7KYK is a Pulse-Eight adapter.

With the NUC7i Intel made BIOS support for CEC, but it is not at all
clear to me if they used CEC tunneling or just hooked up the CEC pin to
some microcontroller.

The only working chipset I have seen is the Parade PS176.


If it really is working on your NUC, then can you add the output of
/sys/kernel/debug/dri/0/i915_display_info?


[root@localhost cec-ctl]# cat /sys/kernel/debug/dri/0/i915_display_info




Connector info
--
connector 48: type DP-1, status: connected
  name:
  physical dimensions: 700x400mm
  subpixel order: Unknown
  CEA rev: 3
  DPCD rev: 12
  audio support: yes
  DP branch device present: yes
  Type: HDMI
  ID: 175IB0
  HW: 1.0
  SW: 7.32
  Max TMDS clock: 60 kHz
  Max bpc: 12


Huh. Based on this document:

https://downloadmirror.intel.com/26061/eng/NUC6i7KYK%20HDMI%202.0%20Firmware%20update%20Instructions.pdf

this is the internal DP-to-HDMI adapter and it has the PS175. So it is a
Parade chipset, and I have seen that work before (at least the PS176).




connector 55: type DP-2, status: connected
  name:
  physical dimensions: 620x340mm
  subpixel order: Unknown
  CEA rev: 3
  DPCD rev: 12
  audio support: yes
  DP branch device present: yes
  Type: HDMI
  ID: BCTRC0
  HW: 2.0
  SW: 0.26


And is this from a USB-C to HDMI adapter? Which one? I don't recognize the ID.

For the record, this is the internal HDMI output of my NUC7i5BNK:

connector 48: type DP-1, status: connected
name:
physical dimensions: 1050x590mm
subpixel order: Unknown
CEA rev: 3
DPCD rev: 12
audio support: yes
DP branch device present: yes
Type: HDMI
ID: MC2800
HW: 2.2
SW: 1.66
Max TMDS clock: 60 kHz
Max bpc: 16

Clearly a Megachip.

Regards,

Hans


Re: [RFC PATCH 7/7] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

2017-05-30 Thread Clint Taylor



On 05/30/2017 09:54 AM, Hans Verkuil wrote:

On 05/30/2017 06:49 PM, Hans Verkuil wrote:

On 05/30/2017 04:19 PM, Clint Taylor wrote:



On 05/30/2017 12:11 AM, Jani Nikula wrote:

On Tue, 30 May 2017, Hans Verkuil  wrote:

On 05/29/2017 09:00 PM, Daniel Vetter wrote:

On Fri, May 26, 2017 at 12:20:48PM +0200, Hans Verkuil wrote:

On 05/26/2017 09:15 AM, Daniel Vetter wrote:

Did you look into also wiring this up for dp mst chains?

Isn't this sufficient? I have no way of testing mst chains.

I think I need some pointers from you, since I am a complete 
newbie when it

comes to mst.
I don't really have more clue, but yeah if you don't have an mst 
thing (a
simple dp port multiplexer is what I use for testing here, then 
plug in a
converter dongle or cable into that) then probably better to not 
wire up

the code for it.
I think my NUC already uses mst internally. But I was planning on 
buying a
dp multiplexer to make sure there is nothing special I need to do 
for mst.


The CEC Tunneling is all in the branch device, so if I understand 
things

correctly it is not affected by mst.

BTW, I did a bit more testing on my NUC7i5BNK: for the HDMI output 
they

use a MegaChip MCDP2800 DP-to-HDMI converter which according to their
datasheet is supposed to implement CEC Tunneling, but if they do 
it is not
exposed as a capability. I'm not sure if it is a MegaChip firmware 
issue
or something else. The BIOS is able to do some CEC, but whether 
they hook
into the MegaChip or have the CEC pin connected to a GPIO or 
something and

have their own controller is something I do not know.

If anyone can clarify what Intel did on the NUC, then that would 
be very

helpful.

It's called LSPCON, see i915/intel_lspcon.c, basically to support HDMI
2.0. Currently we only use it in PCON mode, shows up as DP for us. It
could be used in LS mode, showing up as HDMI 1.4, but we don't support
that in i915.

I don't know about the CEC on that.


My NUC6i7KYK has the MCDP2850 LSPCON and it does support CEC over Aux.
The release notes for the NUC state that there is a BIOS configuration
option for enabling support. My doesn't have the option but the LSPCON
fully supports CEC.


What is the output of:

dd if=/dev/drm_dp_aux0 of=aux0 skip=12288 ibs=1 count=48
od -t x1 aux0

Assuming drm_dp_aux0 is the aux channel for the HDMI output on your NUC.

If the first byte is != 0x00, then it advertises CEC over Aux.

For me it says 0x00.

When you say "it does support CEC over Aux", does that mean you have 
actually
tested it somehow? The only working solution I have seen mentioned 
for the

NUC6i7KYK is a Pulse-Eight adapter.

With the NUC7i Intel made BIOS support for CEC, but it is not at all
clear to me if they used CEC tunneling or just hooked up the CEC pin to
some microcontroller.

The only working chipset I have seen is the Parade PS176.


If it really is working on your NUC, then can you add the output of
/sys/kernel/debug/dri/0/i915_display_info?


[root@localhost cec-ctl]# cat /sys/kernel/debug/dri/0/i915_display_info
CRTC info
-
CRTC 32: pipe: A, active=yes, (size=1920x1080), dither=no, bpp=24
fb: 115, pos: 0x0, size: 3840x2160
encoder 47: type: DDI B, connectors:
connector 48: type: DP-1, status: connected, mode:
id 0:"1920x1080" freq 60 clock 148500 hdisp 1920 hss 2008 hse 
2052 htot 2200 vdisp 1080 vss 1084 vse 1089 vtot 1125 type 0x48 flags 0x5

cursor visible? no, position (0, 0), size 0x0, addr 0x
num_scalers=2, scaler_users=0 scaler_id=-1, scalers[0]: use=no, 
mode=0, scalers[1]: use=no, mode=0
--Plane id 26: type=PRI, crtc_pos=   0x   0, crtc_size=1920x1080, 
src_pos=0.x0., src_size=1920.x1080., format=XR24 
little-endian (0x34325258), rotation=0 (0x0001)
--Plane id 28: type=OVL, crtc_pos=   0x   0, crtc_size=   0x 0, 
src_pos=0.x0., src_size=0.x0., format=N/A, rotation=0 
(0x0001)
--Plane id 30: type=CUR, crtc_pos=   0x   0, crtc_size=   0x 0, 
src_pos=0.x0., src_size=0.x0., format=N/A, rotation=0 
(0x0001)

underrun reporting: cpu=yes pch=yes
CRTC 39: pipe: B, active=yes, (size=3840x2160), dither=no, bpp=36
fb: 115, pos: 0x0, size: 3840x2160
encoder 54: type: DDI C, connectors:
connector 55: type: DP-2, status: connected, mode:
id 0:"3840x2160" freq 30 clock 296703 hdisp 3840 hss 4016 hse 
4104 htot 4400 vdisp 2160 vss 2168 vse 2178 vtot 2250 type 0x48 flags 0x5

cursor visible? no, position (0, 0), size 0x0, addr 0x
num_scalers=2, scaler_users=0 scaler_id=-1, scalers[0]: use=no, 
mode=0, scalers[1]: use=no, mode=0
--Plane id 33: type=PRI, crtc_pos=   0x   0, crtc_size=3840x2160, 
src_pos=0.x0., src_size=3840.x2160., format=XR24 
little-endian (0x34325258), rotation=0 (0x0001)
--Plane id 35: type=OVL, crtc_pos=   0x   0, crtc_size=   0x 0, 
src_pos=0.x0., src_size=0.x0., format=N/A, rotation=0 

Re: [RFC PATCH 7/7] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

2017-05-30 Thread Clint Taylor



On 05/30/2017 09:49 AM, Hans Verkuil wrote:

On 05/30/2017 04:19 PM, Clint Taylor wrote:



On 05/30/2017 12:11 AM, Jani Nikula wrote:

On Tue, 30 May 2017, Hans Verkuil  wrote:

On 05/29/2017 09:00 PM, Daniel Vetter wrote:

On Fri, May 26, 2017 at 12:20:48PM +0200, Hans Verkuil wrote:

On 05/26/2017 09:15 AM, Daniel Vetter wrote:

Did you look into also wiring this up for dp mst chains?

Isn't this sufficient? I have no way of testing mst chains.

I think I need some pointers from you, since I am a complete 
newbie when it

comes to mst.
I don't really have more clue, but yeah if you don't have an mst 
thing (a
simple dp port multiplexer is what I use for testing here, then 
plug in a
converter dongle or cable into that) then probably better to not 
wire up

the code for it.
I think my NUC already uses mst internally. But I was planning on 
buying a
dp multiplexer to make sure there is nothing special I need to do 
for mst.


The CEC Tunneling is all in the branch device, so if I understand 
things

correctly it is not affected by mst.

BTW, I did a bit more testing on my NUC7i5BNK: for the HDMI output 
they

use a MegaChip MCDP2800 DP-to-HDMI converter which according to their
datasheet is supposed to implement CEC Tunneling, but if they do it 
is not
exposed as a capability. I'm not sure if it is a MegaChip firmware 
issue
or something else. The BIOS is able to do some CEC, but whether 
they hook
into the MegaChip or have the CEC pin connected to a GPIO or 
something and

have their own controller is something I do not know.

If anyone can clarify what Intel did on the NUC, then that would be 
very

helpful.

It's called LSPCON, see i915/intel_lspcon.c, basically to support HDMI
2.0. Currently we only use it in PCON mode, shows up as DP for us. It
could be used in LS mode, showing up as HDMI 1.4, but we don't support
that in i915.

I don't know about the CEC on that.


My NUC6i7KYK has the MCDP2850 LSPCON and it does support CEC over Aux.
The release notes for the NUC state that there is a BIOS configuration
option for enabling support. My doesn't have the option but the LSPCON
fully supports CEC.


What is the output of:

dd if=/dev/drm_dp_aux0 of=aux0 skip=12288 ibs=1 count=48
od -t x1 aux0

Assuming drm_dp_aux0 is the aux channel for the HDMI output on your NUC.

If the first byte is != 0x00, then it advertises CEC over Aux.

For me it says 0x00.

slightly different command, but it still dumps DPCD 0x3000 for 48 bytes.

sudo dd if=/dev/drm_dp_aux0 bs=1 skip=12288 count=48 | hexdump -C
  07 00 00 00 00 00 00 00  00 00 00 00 00 00 00 80 
||
0010  f8 23 c4 8f 06 d8 59 7b  37 bb 1e 14 9c cb cd 88 
|.#Y{7...|
0020  4e 84 10 00 04 00 f7 f5  e2 fa a3 30 ad 42 ed 19 
|N..0.B..|





When you say "it does support CEC over Aux", does that mean you have 
actually
tested it somehow? The only working solution I have seen mentioned for 
the

NUC6i7KYK is a Pulse-Eight adapter.

With the NUC7i Intel made BIOS support for CEC, but it is not at all
clear to me if they used CEC tunneling or just hooked up the CEC pin to
some microcontroller.

The only working chipset I have seen is the Parade PS176.
I have a couple PS176 based devices that I purchased from Amazon that do 
not work even though they advertise support at DPCD 0x3000.


Club 3D USB-C->HDMI 2.0 UHD
UptabUSB-C->HDMI 2.0

-Clint




Regards,

Hans



-Clint




BR,
Jani.

It would be so nice to get MegaChip CEC Tunneling working on the 
NUC, because
then you have native CEC support without requiring any Pulse Eight 
adapter.


And add a CEC-capable USB-C to HDMI adapter and you have it on the 
USB-C

output as well.

Regards,

Hans








Re: [RFC PATCH 7/7] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

2017-05-30 Thread Hans Verkuil

On 05/30/2017 06:49 PM, Hans Verkuil wrote:

On 05/30/2017 04:19 PM, Clint Taylor wrote:



On 05/30/2017 12:11 AM, Jani Nikula wrote:

On Tue, 30 May 2017, Hans Verkuil  wrote:

On 05/29/2017 09:00 PM, Daniel Vetter wrote:

On Fri, May 26, 2017 at 12:20:48PM +0200, Hans Verkuil wrote:

On 05/26/2017 09:15 AM, Daniel Vetter wrote:

Did you look into also wiring this up for dp mst chains?

Isn't this sufficient? I have no way of testing mst chains.

I think I need some pointers from you, since I am a complete newbie when it
comes to mst.

I don't really have more clue, but yeah if you don't have an mst thing (a
simple dp port multiplexer is what I use for testing here, then plug in a
converter dongle or cable into that) then probably better to not wire up
the code for it.

I think my NUC already uses mst internally. But I was planning on buying a
dp multiplexer to make sure there is nothing special I need to do for mst.

The CEC Tunneling is all in the branch device, so if I understand things
correctly it is not affected by mst.

BTW, I did a bit more testing on my NUC7i5BNK: for the HDMI output they
use a MegaChip MCDP2800 DP-to-HDMI converter which according to their
datasheet is supposed to implement CEC Tunneling, but if they do it is not
exposed as a capability. I'm not sure if it is a MegaChip firmware issue
or something else. The BIOS is able to do some CEC, but whether they hook
into the MegaChip or have the CEC pin connected to a GPIO or something and
have their own controller is something I do not know.

If anyone can clarify what Intel did on the NUC, then that would be very
helpful.

It's called LSPCON, see i915/intel_lspcon.c, basically to support HDMI
2.0. Currently we only use it in PCON mode, shows up as DP for us. It
could be used in LS mode, showing up as HDMI 1.4, but we don't support
that in i915.

I don't know about the CEC on that.


My NUC6i7KYK has the MCDP2850 LSPCON and it does support CEC over Aux.
The release notes for the NUC state that there is a BIOS configuration
option for enabling support. My doesn't have the option but the LSPCON
fully supports CEC.


What is the output of:

dd if=/dev/drm_dp_aux0 of=aux0 skip=12288 ibs=1 count=48
od -t x1 aux0

Assuming drm_dp_aux0 is the aux channel for the HDMI output on your NUC.

If the first byte is != 0x00, then it advertises CEC over Aux.

For me it says 0x00.

When you say "it does support CEC over Aux", does that mean you have actually
tested it somehow? The only working solution I have seen mentioned for the
NUC6i7KYK is a Pulse-Eight adapter.

With the NUC7i Intel made BIOS support for CEC, but it is not at all
clear to me if they used CEC tunneling or just hooked up the CEC pin to
some microcontroller.

The only working chipset I have seen is the Parade PS176.


If it really is working on your NUC, then can you add the output of
/sys/kernel/debug/dri/0/i915_display_info?

Thanks,

Hans


Re: [RFC PATCH 7/7] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

2017-05-30 Thread Hans Verkuil

On 05/30/2017 04:19 PM, Clint Taylor wrote:



On 05/30/2017 12:11 AM, Jani Nikula wrote:

On Tue, 30 May 2017, Hans Verkuil  wrote:

On 05/29/2017 09:00 PM, Daniel Vetter wrote:

On Fri, May 26, 2017 at 12:20:48PM +0200, Hans Verkuil wrote:

On 05/26/2017 09:15 AM, Daniel Vetter wrote:

Did you look into also wiring this up for dp mst chains?

Isn't this sufficient? I have no way of testing mst chains.

I think I need some pointers from you, since I am a complete newbie when it
comes to mst.

I don't really have more clue, but yeah if you don't have an mst thing (a
simple dp port multiplexer is what I use for testing here, then plug in a
converter dongle or cable into that) then probably better to not wire up
the code for it.

I think my NUC already uses mst internally. But I was planning on buying a
dp multiplexer to make sure there is nothing special I need to do for mst.

The CEC Tunneling is all in the branch device, so if I understand things
correctly it is not affected by mst.

BTW, I did a bit more testing on my NUC7i5BNK: for the HDMI output they
use a MegaChip MCDP2800 DP-to-HDMI converter which according to their
datasheet is supposed to implement CEC Tunneling, but if they do it is not
exposed as a capability. I'm not sure if it is a MegaChip firmware issue
or something else. The BIOS is able to do some CEC, but whether they hook
into the MegaChip or have the CEC pin connected to a GPIO or something and
have their own controller is something I do not know.

If anyone can clarify what Intel did on the NUC, then that would be very
helpful.

It's called LSPCON, see i915/intel_lspcon.c, basically to support HDMI
2.0. Currently we only use it in PCON mode, shows up as DP for us. It
could be used in LS mode, showing up as HDMI 1.4, but we don't support
that in i915.

I don't know about the CEC on that.


My NUC6i7KYK has the MCDP2850 LSPCON and it does support CEC over Aux.
The release notes for the NUC state that there is a BIOS configuration
option for enabling support. My doesn't have the option but the LSPCON
fully supports CEC.


What is the output of:

dd if=/dev/drm_dp_aux0 of=aux0 skip=12288 ibs=1 count=48
od -t x1 aux0

Assuming drm_dp_aux0 is the aux channel for the HDMI output on your NUC.

If the first byte is != 0x00, then it advertises CEC over Aux.

For me it says 0x00.

When you say "it does support CEC over Aux", does that mean you have actually
tested it somehow? The only working solution I have seen mentioned for the
NUC6i7KYK is a Pulse-Eight adapter.

With the NUC7i Intel made BIOS support for CEC, but it is not at all
clear to me if they used CEC tunneling or just hooked up the CEC pin to
some microcontroller.

The only working chipset I have seen is the Parade PS176.

Regards,

Hans



-Clint




BR,
Jani.


It would be so nice to get MegaChip CEC Tunneling working on the NUC, because
then you have native CEC support without requiring any Pulse Eight adapter.

And add a CEC-capable USB-C to HDMI adapter and you have it on the USB-C
output as well.

Regards,

Hans






Re: [RFC PATCH 7/7] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

2017-05-30 Thread Clint Taylor



On 05/30/2017 12:11 AM, Jani Nikula wrote:

On Tue, 30 May 2017, Hans Verkuil  wrote:

On 05/29/2017 09:00 PM, Daniel Vetter wrote:

On Fri, May 26, 2017 at 12:20:48PM +0200, Hans Verkuil wrote:

On 05/26/2017 09:15 AM, Daniel Vetter wrote:

Did you look into also wiring this up for dp mst chains?

Isn't this sufficient? I have no way of testing mst chains.

I think I need some pointers from you, since I am a complete newbie when it
comes to mst.

I don't really have more clue, but yeah if you don't have an mst thing (a
simple dp port multiplexer is what I use for testing here, then plug in a
converter dongle or cable into that) then probably better to not wire up
the code for it.

I think my NUC already uses mst internally. But I was planning on buying a
dp multiplexer to make sure there is nothing special I need to do for mst.

The CEC Tunneling is all in the branch device, so if I understand things
correctly it is not affected by mst.

BTW, I did a bit more testing on my NUC7i5BNK: for the HDMI output they
use a MegaChip MCDP2800 DP-to-HDMI converter which according to their
datasheet is supposed to implement CEC Tunneling, but if they do it is not
exposed as a capability. I'm not sure if it is a MegaChip firmware issue
or something else. The BIOS is able to do some CEC, but whether they hook
into the MegaChip or have the CEC pin connected to a GPIO or something and
have their own controller is something I do not know.

If anyone can clarify what Intel did on the NUC, then that would be very
helpful.

It's called LSPCON, see i915/intel_lspcon.c, basically to support HDMI
2.0. Currently we only use it in PCON mode, shows up as DP for us. It
could be used in LS mode, showing up as HDMI 1.4, but we don't support
that in i915.

I don't know about the CEC on that.


My NUC6i7KYK has the MCDP2850 LSPCON and it does support CEC over Aux. 
The release notes for the NUC state that there is a BIOS configuration 
option for enabling support. My doesn't have the option but the LSPCON 
fully supports CEC.


-Clint




BR,
Jani.


It would be so nice to get MegaChip CEC Tunneling working on the NUC, because
then you have native CEC support without requiring any Pulse Eight adapter.

And add a CEC-capable USB-C to HDMI adapter and you have it on the USB-C
output as well.

Regards,

Hans




Re: [RFC PATCH 7/7] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

2017-05-30 Thread Jani Nikula
On Tue, 30 May 2017, Hans Verkuil  wrote:
> On 05/29/2017 09:00 PM, Daniel Vetter wrote:
>> On Fri, May 26, 2017 at 12:20:48PM +0200, Hans Verkuil wrote:
>>> On 05/26/2017 09:15 AM, Daniel Vetter wrote:
 Did you look into also wiring this up for dp mst chains?
>>>
>>> Isn't this sufficient? I have no way of testing mst chains.
>>>
>>> I think I need some pointers from you, since I am a complete newbie when it
>>> comes to mst.
>> 
>> I don't really have more clue, but yeah if you don't have an mst thing (a
>> simple dp port multiplexer is what I use for testing here, then plug in a
>> converter dongle or cable into that) then probably better to not wire up
>> the code for it.
>
> I think my NUC already uses mst internally. But I was planning on buying a
> dp multiplexer to make sure there is nothing special I need to do for mst.
>
> The CEC Tunneling is all in the branch device, so if I understand things
> correctly it is not affected by mst.
>
> BTW, I did a bit more testing on my NUC7i5BNK: for the HDMI output they
> use a MegaChip MCDP2800 DP-to-HDMI converter which according to their
> datasheet is supposed to implement CEC Tunneling, but if they do it is not
> exposed as a capability. I'm not sure if it is a MegaChip firmware issue
> or something else. The BIOS is able to do some CEC, but whether they hook
> into the MegaChip or have the CEC pin connected to a GPIO or something and
> have their own controller is something I do not know.
>
> If anyone can clarify what Intel did on the NUC, then that would be very
> helpful.

It's called LSPCON, see i915/intel_lspcon.c, basically to support HDMI
2.0. Currently we only use it in PCON mode, shows up as DP for us. It
could be used in LS mode, showing up as HDMI 1.4, but we don't support
that in i915.

I don't know about the CEC on that.


BR,
Jani.

>
> It would be so nice to get MegaChip CEC Tunneling working on the NUC, because
> then you have native CEC support without requiring any Pulse Eight adapter.
>
> And add a CEC-capable USB-C to HDMI adapter and you have it on the USB-C
> output as well.
>
> Regards,
>
>   Hans

-- 
Jani Nikula, Intel Open Source Technology Center


Re: [RFC PATCH 7/7] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

2017-05-30 Thread Hans Verkuil

On 05/29/2017 09:00 PM, Daniel Vetter wrote:

On Fri, May 26, 2017 at 12:20:48PM +0200, Hans Verkuil wrote:

On 05/26/2017 09:15 AM, Daniel Vetter wrote:

Did you look into also wiring this up for dp mst chains?


Isn't this sufficient? I have no way of testing mst chains.

I think I need some pointers from you, since I am a complete newbie when it
comes to mst.


I don't really have more clue, but yeah if you don't have an mst thing (a
simple dp port multiplexer is what I use for testing here, then plug in a
converter dongle or cable into that) then probably better to not wire up
the code for it.


I think my NUC already uses mst internally. But I was planning on buying a
dp multiplexer to make sure there is nothing special I need to do for mst.

The CEC Tunneling is all in the branch device, so if I understand things
correctly it is not affected by mst.

BTW, I did a bit more testing on my NUC7i5BNK: for the HDMI output they
use a MegaChip MCDP2800 DP-to-HDMI converter which according to their
datasheet is supposed to implement CEC Tunneling, but if they do it is not
exposed as a capability. I'm not sure if it is a MegaChip firmware issue
or something else. The BIOS is able to do some CEC, but whether they hook
into the MegaChip or have the CEC pin connected to a GPIO or something and
have their own controller is something I do not know.

If anyone can clarify what Intel did on the NUC, then that would be very
helpful.

It would be so nice to get MegaChip CEC Tunneling working on the NUC, because
then you have native CEC support without requiring any Pulse Eight adapter.

And add a CEC-capable USB-C to HDMI adapter and you have it on the USB-C
output as well.

Regards,

Hans


Re: [RFC PATCH 7/7] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

2017-05-29 Thread Daniel Vetter
On Fri, May 26, 2017 at 12:20:48PM +0200, Hans Verkuil wrote:
> On 05/26/2017 09:15 AM, Daniel Vetter wrote:
> > On Thu, May 25, 2017 at 05:06:26PM +0200, Hans Verkuil wrote:
> >> From: Hans Verkuil 
> >>
> >> Implement support for this DisplayPort feature.
> >>
> >> The cec device is created whenever it detects an adapter that
> >> has this feature. It is only removed when a new adapter is connected
> >> that does not support this. If a new adapter is connected that has
> >> different properties than the previous one, then the old cec device is
> >> unregistered and a new one is registered to replace the old one.
> >>
> >> Signed-off-by: Hans Verkuil 
> > 
> > Some small comments below.
> > 
> >> ---
> >>  drivers/gpu/drm/i915/Kconfig| 11 ++
> >>  drivers/gpu/drm/i915/intel_dp.c | 46 
> >> +
> >>  2 files changed, 53 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> >> index a5cd5dacf055..f317b13a1409 100644
> >> --- a/drivers/gpu/drm/i915/Kconfig
> >> +++ b/drivers/gpu/drm/i915/Kconfig
> >> @@ -124,6 +124,17 @@ config DRM_I915_GVT_KVMGT
> >>  Choose this option if you want to enable KVMGT support for
> >>  Intel GVT-g.
> >>  
> >> +config DRM_I915_DP_CEC
> >> +  tristate "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
> >> +  depends on DRM_I915 && CEC_CORE
> >> +  select DRM_DP_CEC
> >> +  help
> >> +Choose this option if you want to enable HDMI CEC support for
> >> +DisplayPort/USB-C to HDMI adapters.
> >> +
> >> +Note: not all adapters support this feature, and even for those
> >> +that do support this often do not hook up the CEC pin.
> > 
> > Why Kconfig? There's not anything else optional in i915.ko (except debug
> > stuff ofc), since generally just not worth the pain. Also doesn't seem to
> > be wired up at all :-)
> 
> It selects DRM_DP_CEC, but you're right, it can be dropped.
> 
> > 
> >> +
> >>  menu "drm/i915 Debugging"
> >>  depends on DRM_I915
> >>  depends on EXPERT
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> >> b/drivers/gpu/drm/i915/intel_dp.c
> >> index ee77b519835c..38e17ee2548d 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -32,6 +32,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>  #include 
> >>  #include 
> >>  #include 
> >> @@ -1405,6 +1406,7 @@ static void intel_aux_reg_init(struct intel_dp 
> >> *intel_dp)
> >>  static void
> >>  intel_dp_aux_fini(struct intel_dp *intel_dp)
> >>  {
> >> +  cec_unregister_adapter(intel_dp->aux.cec_adap);
> >>kfree(intel_dp->aux.name);
> >>  }
> >>  
> >> @@ -4179,6 +4181,33 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
> >>return -EINVAL;
> >>  }
> >>  
> >> +static bool
> >> +intel_dp_check_cec_status(struct intel_dp *intel_dp)
> >> +{
> >> +  bool handled = false;
> >> +
> >> +  for (;;) {
> >> +  u8 cec_irq;
> >> +  int ret;
> >> +
> >> +  ret = drm_dp_dpcd_readb(_dp->aux,
> >> +  DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
> >> +  _irq);
> >> +  if (ret < 0 || !(cec_irq & DP_CEC_IRQ))
> >> +  return handled;
> >> +
> >> +  cec_irq &= ~DP_CEC_IRQ;
> >> +  drm_dp_cec_irq(_dp->aux);
> >> +  handled = true;
> >> +
> >> +  ret = drm_dp_dpcd_writeb(_dp->aux,
> >> +   DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
> >> +   cec_irq);
> >> +  if (ret < 0)
> >> +  return handled;
> >> +  }
> >> +}
> > 
> > Shouldn't the above be a helper in the cec library? Doesn't look i915
> > specific to me at least ...
> 
> Good point, this can be moved to drm_dp_cec_irq().
> 
> > 
> >> +
> >>  static void
> >>  intel_dp_retrain_link(struct intel_dp *intel_dp)
> >>  {
> >> @@ -4553,6 +4582,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
> >>intel_dp->has_audio = intel_dp->force_audio == HDMI_AUDIO_ON;
> >>else
> >>intel_dp->has_audio = drm_detect_monitor_audio(edid);
> >> +  cec_s_phys_addr_from_edid(intel_dp->aux.cec_adap, edid);
> >>  }
> >>  
> >>  static void
> >> @@ -4562,6 +4592,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> >>  
> >>kfree(intel_connector->detect_edid);
> >>intel_connector->detect_edid = NULL;
> >> +  cec_phys_addr_invalidate(intel_dp->aux.cec_adap);
> >>  
> >>intel_dp->has_audio = false;
> >>  }
> >> @@ -4582,13 +4613,17 @@ intel_dp_long_pulse(struct intel_connector 
> >> *intel_connector)
> >>intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
> >>  
> >>/* Can't disconnect eDP, but you can close the lid... */
> >> -  if (is_edp(intel_dp))
> >> +  if (is_edp(intel_dp)) {
> >>status = edp_detect(intel_dp);
> >> -  else if 

Re: [RFC PATCH 7/7] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

2017-05-26 Thread Hans Verkuil
On 05/26/2017 09:15 AM, Daniel Vetter wrote:
> On Thu, May 25, 2017 at 05:06:26PM +0200, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Implement support for this DisplayPort feature.
>>
>> The cec device is created whenever it detects an adapter that
>> has this feature. It is only removed when a new adapter is connected
>> that does not support this. If a new adapter is connected that has
>> different properties than the previous one, then the old cec device is
>> unregistered and a new one is registered to replace the old one.
>>
>> Signed-off-by: Hans Verkuil 
> 
> Some small comments below.
> 
>> ---
>>  drivers/gpu/drm/i915/Kconfig| 11 ++
>>  drivers/gpu/drm/i915/intel_dp.c | 46 
>> +
>>  2 files changed, 53 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
>> index a5cd5dacf055..f317b13a1409 100644
>> --- a/drivers/gpu/drm/i915/Kconfig
>> +++ b/drivers/gpu/drm/i915/Kconfig
>> @@ -124,6 +124,17 @@ config DRM_I915_GVT_KVMGT
>>Choose this option if you want to enable KVMGT support for
>>Intel GVT-g.
>>  
>> +config DRM_I915_DP_CEC
>> +tristate "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
>> +depends on DRM_I915 && CEC_CORE
>> +select DRM_DP_CEC
>> +help
>> +  Choose this option if you want to enable HDMI CEC support for
>> +  DisplayPort/USB-C to HDMI adapters.
>> +
>> +  Note: not all adapters support this feature, and even for those
>> +  that do support this often do not hook up the CEC pin.
> 
> Why Kconfig? There's not anything else optional in i915.ko (except debug
> stuff ofc), since generally just not worth the pain. Also doesn't seem to
> be wired up at all :-)

It selects DRM_DP_CEC, but you're right, it can be dropped.

> 
>> +
>>  menu "drm/i915 Debugging"
>>  depends on DRM_I915
>>  depends on EXPERT
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index ee77b519835c..38e17ee2548d 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -32,6 +32,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -1405,6 +1406,7 @@ static void intel_aux_reg_init(struct intel_dp 
>> *intel_dp)
>>  static void
>>  intel_dp_aux_fini(struct intel_dp *intel_dp)
>>  {
>> +cec_unregister_adapter(intel_dp->aux.cec_adap);
>>  kfree(intel_dp->aux.name);
>>  }
>>  
>> @@ -4179,6 +4181,33 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
>>  return -EINVAL;
>>  }
>>  
>> +static bool
>> +intel_dp_check_cec_status(struct intel_dp *intel_dp)
>> +{
>> +bool handled = false;
>> +
>> +for (;;) {
>> +u8 cec_irq;
>> +int ret;
>> +
>> +ret = drm_dp_dpcd_readb(_dp->aux,
>> +DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
>> +_irq);
>> +if (ret < 0 || !(cec_irq & DP_CEC_IRQ))
>> +return handled;
>> +
>> +cec_irq &= ~DP_CEC_IRQ;
>> +drm_dp_cec_irq(_dp->aux);
>> +handled = true;
>> +
>> +ret = drm_dp_dpcd_writeb(_dp->aux,
>> + DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
>> + cec_irq);
>> +if (ret < 0)
>> +return handled;
>> +}
>> +}
> 
> Shouldn't the above be a helper in the cec library? Doesn't look i915
> specific to me at least ...

Good point, this can be moved to drm_dp_cec_irq().

> 
>> +
>>  static void
>>  intel_dp_retrain_link(struct intel_dp *intel_dp)
>>  {
>> @@ -4553,6 +4582,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
>>  intel_dp->has_audio = intel_dp->force_audio == HDMI_AUDIO_ON;
>>  else
>>  intel_dp->has_audio = drm_detect_monitor_audio(edid);
>> +cec_s_phys_addr_from_edid(intel_dp->aux.cec_adap, edid);
>>  }
>>  
>>  static void
>> @@ -4562,6 +4592,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>>  
>>  kfree(intel_connector->detect_edid);
>>  intel_connector->detect_edid = NULL;
>> +cec_phys_addr_invalidate(intel_dp->aux.cec_adap);
>>  
>>  intel_dp->has_audio = false;
>>  }
>> @@ -4582,13 +4613,17 @@ intel_dp_long_pulse(struct intel_connector 
>> *intel_connector)
>>  intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
>>  
>>  /* Can't disconnect eDP, but you can close the lid... */
>> -if (is_edp(intel_dp))
>> +if (is_edp(intel_dp)) {
>>  status = edp_detect(intel_dp);
>> -else if (intel_digital_port_connected(to_i915(dev),
>> -  dp_to_dig_port(intel_dp)))
>> +} else if (intel_digital_port_connected(to_i915(dev),
>> +dp_to_dig_port(intel_dp))) {
>>  status = 

Re: [RFC PATCH 7/7] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

2017-05-26 Thread Hans Verkuil
On 05/26/2017 12:13 PM, Jani Nikula wrote:
> On Thu, 25 May 2017, Hans Verkuil  wrote:
>> @@ -4179,6 +4181,33 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
>>  return -EINVAL;
>>  }
>>  
>> +static bool
>> +intel_dp_check_cec_status(struct intel_dp *intel_dp)
>> +{
>> +bool handled = false;
>> +
>> +for (;;) {
>> +u8 cec_irq;
>> +int ret;
>> +
>> +ret = drm_dp_dpcd_readb(_dp->aux,
>> +DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
>> +_irq);
>> +if (ret < 0 || !(cec_irq & DP_CEC_IRQ))
>> +return handled;
>> +
>> +cec_irq &= ~DP_CEC_IRQ;
>> +drm_dp_cec_irq(_dp->aux);
>> +handled = true;
>> +
>> +ret = drm_dp_dpcd_writeb(_dp->aux,
>> + DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
>> + cec_irq);
>> +if (ret < 0)
>> +return handled;
>> +}
> 
> DP sinks suck. Please add a limit to the loop.

Good to know. I wondered about that.

Regards,

Hans


Re: [RFC PATCH 7/7] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

2017-05-26 Thread Jani Nikula
On Thu, 25 May 2017, Hans Verkuil  wrote:
> @@ -4179,6 +4181,33 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
>   return -EINVAL;
>  }
>  
> +static bool
> +intel_dp_check_cec_status(struct intel_dp *intel_dp)
> +{
> + bool handled = false;
> +
> + for (;;) {
> + u8 cec_irq;
> + int ret;
> +
> + ret = drm_dp_dpcd_readb(_dp->aux,
> + DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
> + _irq);
> + if (ret < 0 || !(cec_irq & DP_CEC_IRQ))
> + return handled;
> +
> + cec_irq &= ~DP_CEC_IRQ;
> + drm_dp_cec_irq(_dp->aux);
> + handled = true;
> +
> + ret = drm_dp_dpcd_writeb(_dp->aux,
> +  DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
> +  cec_irq);
> + if (ret < 0)
> + return handled;
> + }

DP sinks suck. Please add a limit to the loop.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center


Re: [RFC PATCH 7/7] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

2017-05-26 Thread Daniel Vetter
On Thu, May 25, 2017 at 05:06:26PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Implement support for this DisplayPort feature.
> 
> The cec device is created whenever it detects an adapter that
> has this feature. It is only removed when a new adapter is connected
> that does not support this. If a new adapter is connected that has
> different properties than the previous one, then the old cec device is
> unregistered and a new one is registered to replace the old one.
> 
> Signed-off-by: Hans Verkuil 

Some small comments below.

> ---
>  drivers/gpu/drm/i915/Kconfig| 11 ++
>  drivers/gpu/drm/i915/intel_dp.c | 46 
> +
>  2 files changed, 53 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index a5cd5dacf055..f317b13a1409 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -124,6 +124,17 @@ config DRM_I915_GVT_KVMGT
> Choose this option if you want to enable KVMGT support for
> Intel GVT-g.
>  
> +config DRM_I915_DP_CEC
> + tristate "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
> + depends on DRM_I915 && CEC_CORE
> + select DRM_DP_CEC
> + help
> +   Choose this option if you want to enable HDMI CEC support for
> +   DisplayPort/USB-C to HDMI adapters.
> +
> +   Note: not all adapters support this feature, and even for those
> +   that do support this often do not hook up the CEC pin.

Why Kconfig? There's not anything else optional in i915.ko (except debug
stuff ofc), since generally just not worth the pain. Also doesn't seem to
be wired up at all :-)

> +
>  menu "drm/i915 Debugging"
>  depends on DRM_I915
>  depends on EXPERT
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ee77b519835c..38e17ee2548d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -32,6 +32,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1405,6 +1406,7 @@ static void intel_aux_reg_init(struct intel_dp 
> *intel_dp)
>  static void
>  intel_dp_aux_fini(struct intel_dp *intel_dp)
>  {
> + cec_unregister_adapter(intel_dp->aux.cec_adap);
>   kfree(intel_dp->aux.name);
>  }
>  
> @@ -4179,6 +4181,33 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
>   return -EINVAL;
>  }
>  
> +static bool
> +intel_dp_check_cec_status(struct intel_dp *intel_dp)
> +{
> + bool handled = false;
> +
> + for (;;) {
> + u8 cec_irq;
> + int ret;
> +
> + ret = drm_dp_dpcd_readb(_dp->aux,
> + DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
> + _irq);
> + if (ret < 0 || !(cec_irq & DP_CEC_IRQ))
> + return handled;
> +
> + cec_irq &= ~DP_CEC_IRQ;
> + drm_dp_cec_irq(_dp->aux);
> + handled = true;
> +
> + ret = drm_dp_dpcd_writeb(_dp->aux,
> +  DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
> +  cec_irq);
> + if (ret < 0)
> + return handled;
> + }
> +}

Shouldn't the above be a helper in the cec library? Doesn't look i915
specific to me at least ...

> +
>  static void
>  intel_dp_retrain_link(struct intel_dp *intel_dp)
>  {
> @@ -4553,6 +4582,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
>   intel_dp->has_audio = intel_dp->force_audio == HDMI_AUDIO_ON;
>   else
>   intel_dp->has_audio = drm_detect_monitor_audio(edid);
> + cec_s_phys_addr_from_edid(intel_dp->aux.cec_adap, edid);
>  }
>  
>  static void
> @@ -4562,6 +4592,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>  
>   kfree(intel_connector->detect_edid);
>   intel_connector->detect_edid = NULL;
> + cec_phys_addr_invalidate(intel_dp->aux.cec_adap);
>  
>   intel_dp->has_audio = false;
>  }
> @@ -4582,13 +4613,17 @@ intel_dp_long_pulse(struct intel_connector 
> *intel_connector)
>   intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
>  
>   /* Can't disconnect eDP, but you can close the lid... */
> - if (is_edp(intel_dp))
> + if (is_edp(intel_dp)) {
>   status = edp_detect(intel_dp);
> - else if (intel_digital_port_connected(to_i915(dev),
> -   dp_to_dig_port(intel_dp)))
> + } else if (intel_digital_port_connected(to_i915(dev),
> + dp_to_dig_port(intel_dp))) {
>   status = intel_dp_detect_dpcd(intel_dp);
> - else
> + if (status == connector_status_connected)
> + drm_dp_cec_configure_adapter(_dp->aux,
> +  intel_dp->aux.name, dev->dev);

Did you look into also 

[RFC PATCH 7/7] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

2017-05-25 Thread Hans Verkuil
From: Hans Verkuil 

Implement support for this DisplayPort feature.

The cec device is created whenever it detects an adapter that
has this feature. It is only removed when a new adapter is connected
that does not support this. If a new adapter is connected that has
different properties than the previous one, then the old cec device is
unregistered and a new one is registered to replace the old one.

Signed-off-by: Hans Verkuil 
---
 drivers/gpu/drm/i915/Kconfig| 11 ++
 drivers/gpu/drm/i915/intel_dp.c | 46 +
 2 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index a5cd5dacf055..f317b13a1409 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -124,6 +124,17 @@ config DRM_I915_GVT_KVMGT
  Choose this option if you want to enable KVMGT support for
  Intel GVT-g.
 
+config DRM_I915_DP_CEC
+   tristate "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
+   depends on DRM_I915 && CEC_CORE
+   select DRM_DP_CEC
+   help
+ Choose this option if you want to enable HDMI CEC support for
+ DisplayPort/USB-C to HDMI adapters.
+
+ Note: not all adapters support this feature, and even for those
+ that do support this often do not hook up the CEC pin.
+
 menu "drm/i915 Debugging"
 depends on DRM_I915
 depends on EXPERT
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ee77b519835c..38e17ee2548d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1405,6 +1406,7 @@ static void intel_aux_reg_init(struct intel_dp *intel_dp)
 static void
 intel_dp_aux_fini(struct intel_dp *intel_dp)
 {
+   cec_unregister_adapter(intel_dp->aux.cec_adap);
kfree(intel_dp->aux.name);
 }
 
@@ -4179,6 +4181,33 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
return -EINVAL;
 }
 
+static bool
+intel_dp_check_cec_status(struct intel_dp *intel_dp)
+{
+   bool handled = false;
+
+   for (;;) {
+   u8 cec_irq;
+   int ret;
+
+   ret = drm_dp_dpcd_readb(_dp->aux,
+   DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
+   _irq);
+   if (ret < 0 || !(cec_irq & DP_CEC_IRQ))
+   return handled;
+
+   cec_irq &= ~DP_CEC_IRQ;
+   drm_dp_cec_irq(_dp->aux);
+   handled = true;
+
+   ret = drm_dp_dpcd_writeb(_dp->aux,
+DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
+cec_irq);
+   if (ret < 0)
+   return handled;
+   }
+}
+
 static void
 intel_dp_retrain_link(struct intel_dp *intel_dp)
 {
@@ -4553,6 +4582,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
intel_dp->has_audio = intel_dp->force_audio == HDMI_AUDIO_ON;
else
intel_dp->has_audio = drm_detect_monitor_audio(edid);
+   cec_s_phys_addr_from_edid(intel_dp->aux.cec_adap, edid);
 }
 
 static void
@@ -4562,6 +4592,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
 
kfree(intel_connector->detect_edid);
intel_connector->detect_edid = NULL;
+   cec_phys_addr_invalidate(intel_dp->aux.cec_adap);
 
intel_dp->has_audio = false;
 }
@@ -4582,13 +4613,17 @@ intel_dp_long_pulse(struct intel_connector 
*intel_connector)
intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
 
/* Can't disconnect eDP, but you can close the lid... */
-   if (is_edp(intel_dp))
+   if (is_edp(intel_dp)) {
status = edp_detect(intel_dp);
-   else if (intel_digital_port_connected(to_i915(dev),
- dp_to_dig_port(intel_dp)))
+   } else if (intel_digital_port_connected(to_i915(dev),
+   dp_to_dig_port(intel_dp))) {
status = intel_dp_detect_dpcd(intel_dp);
-   else
+   if (status == connector_status_connected)
+   drm_dp_cec_configure_adapter(_dp->aux,
+intel_dp->aux.name, dev->dev);
+   } else {
status = connector_status_disconnected;
+   }
 
if (status == connector_status_disconnected) {
memset(_dp->compliance, 0, sizeof(intel_dp->compliance));
@@ -5080,6 +5115,9 @@ intel_dp_hpd_pulse(struct intel_digital_port 
*intel_dig_port, bool long_hpd)
 
intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
 
+   if (intel_dp->aux.cec_adap)
+   intel_dp_check_cec_status(intel_dp);
+
if (intel_dp->is_mst) {
if