Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1416428 > unrolled thread
| Started by | Javier Martinez Canillas <javier@osg.samsung.com> |
|---|---|
| First post | 2016-06-07 19:10 +0200 |
| Last post | 2016-06-08 16:30 +0200 |
| Articles | 5 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH v14.1 01/17] drm: bridge: analogix/dp: split exynos dp driver to bridge directory Javier Martinez Canillas <javier@osg.samsung.com> - 2016-06-07 19:10 +0200
Re: [PATCH v14.1 01/17] drm: bridge: analogix/dp: split exynos dp driver to bridge directory Yakir Yang <ykk@rock-chips.com> - 2016-06-08 03:30 +0200
Re: [PATCH v14.1 01/17] drm: bridge: analogix/dp: split exynos dp driver to bridge directory Marc Zyngier <marc.zyngier@arm.com> - 2016-06-08 09:50 +0200
Re: [PATCH v14.1 01/17] drm: bridge: analogix/dp: split exynos dp driver to bridge directory Yakir Yang <ykk@rock-chips.com> - 2016-06-08 13:00 +0200
Re: [PATCH v14.1 01/17] drm: bridge: analogix/dp: split exynos dp driver to bridge directory Javier Martinez Canillas <javier@osg.samsung.com> - 2016-06-08 16:30 +0200
| From | Javier Martinez Canillas <javier@osg.samsung.com> |
|---|---|
| Date | 2016-06-07 19:10 +0200 |
| Subject | Re: [PATCH v14.1 01/17] drm: bridge: analogix/dp: split exynos dp driver to bridge directory |
| Message-ID | <rHsFP-5sl-15@gated-at.bofh.it> |
Hello Yakir, On 03/17/2016 05:47 PM, Heiko Stübner wrote: > Split the dp core driver from exynos directory to bridge directory, > and rename the core driver to analogix_dp_*, rename the platform > code to exynos_dp. > > Beside the new analogix_dp driver would export six hooks. > "analogix_dp_bind()" and "analogix_dp_unbind()" > "analogix_dp_suspned()" and "analogix_dp_resume()" > "analogix_dp_detect()" and "analogix_dp_get_modes()" > > The bind/unbind symbols is used for analogix platform driver to connect > with analogix_dp core driver. And the detect/get_modes is used for analogix > platform driver to init the connector. > > They reason why connector need register in helper driver is rockchip drm > haven't implement the atomic API, but Exynos drm have implement it, so > there would need two different connector helper functions, that's why we > leave the connector register in helper driver. > > Signed-off-by: Yakir Yang <ykk@rock-chips.com> > --- Marc reported that his Exynos5250 Snow Chromebook fails to boot with v4.7-rc. I've done a git bisect and tracked down to this commit. The problem is a NULL pointer dereference to connector->dev in drm_mode_create(connector->dev) when called from exynos_dp_get_modes(). The error log is at [1]. I'm trying to figure out the issue but wanted to mention in case you have any hints about what could be the cause. AFAICT the problem is related to the fact that drm_connector_init() is called in analogix_dp_bridge_attach() and the connector passed as argument is the one in struct analogix_dp_device *dp, but later exynos_dp_get_modes() calls drm_mode_create() passing the connector in struct exynos_dp_device *dp, which has not been previously initialized. [1] [ 4.175676] Unable to handle kernel NULL pointer dereference at virtual address 00000210 [ 4.182793] pgd = c0004000 [ 4.185171] [00000210] *pgd=00000000 [ 4.188657] Internal error: Oops: 5 [#1] PREEMPT SMP ARM [ 4.194164] Modules linked in: [ 4.196964] CPU: 1 PID: 68 Comm: kworker/1:1 Not tainted 4.7.0-rc2-next-20160606-00006-g876c3150327a #97 [ 4.206565] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [ 4.212582] Workqueue: events hdmi_hotplug_work_func [ 4.217383] task: ed758000 ti: ed75c000 task.ti: ed75c000 [ 4.222887] PC is at __mutex_lock_slowpath+0x6c/0x404 [ 4.227913] LR is at preempt_count_add+0xa4/0x140 [ 4.232592] pc : [<c06d21d4>] lr : [<c0139de4>] psr: 60000093 [ 4.232592] sp : ed75dd80 ip : 00000000 fp : ee8c1180 [ 4.244193] r10: 00000300 r9 : ed758000 r8 : 00000001 [ 4.249262] r7 : 60000013 r6 : c0b473a8 r5 : 00000210 r4 : 0000020c [ 4.261955] r3 : 00000000 r2 : 00000001 r1 : ee8c2808 r0 : 00000000 [ 4.273270] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none [ 4.285336] Control: 10c5387d Table: 4000406a DAC: 00000051 [ 4.295885] Process kworker/1:1 (pid: 68, stack limit = 0xed75c210) [ 4.306979] Stack: (0xed75dd80 to 0xed75e000) [ 4.316102] dd80: ee8c1180 c03dd21c ee8f0820 00000001 0000020c 0000020c 00000000 00000000 [ 4.329201] dda0: 00000001 dededede 00000300 c06d2578 ee8c2808 c03d00f4 ee801d80 c0b0261c [ 4.342362] ddc0: 00000041 ee8c2800 00000000 00000001 ee8f0810 ee8f0b65 c074d2b4 c03d35b0 [ 4.353140] dde0: 00000000 00000300 ee8c1180 c03d81d0 c387cf74 ee8f0820 00000001 c03eaffc [ 4.361811] de00: 00000000 ee8f0820 00000001 c03f28bc ee8f0b65 c06d400c ee8c1180 006d40a4 [ 4.370483] de20: 00000000 01820a11 81010300 00000100 00000064 ee8f085c ee8f0820 c0b1f894 [ 4.379150] de40: c3872800 c0b82fac c074d2b4 c03b7550 c0b13274 c0327974 00000001 eefb8530 [ 4.387805] de60: 00000556 00000000 c38729b0 c3872800 ee8c1180 00000001 ee8c7000 00000000 [ 4.396467] de80: 00000556 00000300 c38729b0 00000002 ee8c1180 c03c107c ee8c7000 c3872800 [ 4.405121] dea0: ee8c7240 c3872800 c0b82fac c03c3268 c3872ad4 00000001 c3872ae0 c03b7af0 [ 4.413776] dec0: eefb8000 c0139804 00000000 c0136484 ee8a0000 ee8c1180 c3870b4c eefb7b00 [ 4.422415] dee0: eefbb000 00000000 00000000 eefb7b00 ee8c1180 c012ff2c 00000008 eefb7b24 [ 4.431045] df00: eefb7b00 ee8c1198 00000008 eefb7b24 c0b02100 ed75c000 eefb7b00 c0130178 [ 4.439664] df20: ed75c000 ee863040 ee8c1180 00000000 ee863040 ee8c1180 c0130140 00000000 [ 4.448274] df40: 00000000 00000000 00000000 c0135454 00000000 00000000 00000000 ee8c1180 [ 4.456879] df60: 00000000 00000000 dead4ead ffffffff ffffffff ed75df74 ed75df74 00000000 [ 4.465462] df80: 00000000 dead4ead ffffffff ffffffff ed75df90 ed75df90 ed75dfac ee863040 [ 4.474028] dfa0: c0135378 00000000 00000000 c0107978 00000000 00000000 00000000 00000000 [ 4.482597] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 4.491133] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000 [ 4.499643] [<c06d21d4>] (__mutex_lock_slowpath) from [<c06d2578>] (mutex_lock+0xc/0x24) [ 4.507102] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready [ 4.514264] [<c06d2578>] (mutex_lock) from [<c03d00f4>] (drm_mode_object_get_reg+0x28/0x80) [ 4.522981] [<c03d00f4>] (drm_mode_object_get_reg) from [<c03d35b0>] (drm_mode_object_get+0x18/0x20) [ 4.532491] [<c03d35b0>] (drm_mode_object_get) from [<c03d81d0>] (drm_mode_create+0x3c/0x58) [ 4.541316] [<c03d81d0>] (drm_mode_create) from [<c03eaffc>] (exynos_dp_get_modes+0x24/0x80) [ 4.550160] [<c03eaffc>] (exynos_dp_get_modes) from [<c03f28bc>] (analogix_dp_get_modes+0x198/0x278) [ 4.559735] [<c03f28bc>] (analogix_dp_get_modes) from [<c03b7550>] (drm_helper_probe_single_connector_modes+0x114/0x554) [ 4.571078] [<c03b7550>] (drm_helper_probe_single_connector_modes) from [<c03c107c>] (drm_fb_helper_probe_connector_modes+0x48/0x68) [ 4.583513] [<c03c107c>] (drm_fb_helper_probe_connector_modes) from [<c03c3268>] (drm_fb_helper_hotplug_event+0x70/0xdc) [ 4.594921] [<c03c3268>] (drm_fb_helper_hotplug_event) from [<c03b7af0>] (drm_helper_hpd_irq_event+0xd4/0x160) [ 4.605474] [<c03b7af0>] (drm_helper_hpd_irq_event) from [<c012ff2c>] (process_one_work+0x124/0x338) [ 4.615144] [<c012ff2c>] (process_one_work) from [<c0130178>] (worker_thread+0x38/0x4d4) [ 4.623764] [<c0130178>] (worker_thread) from [<c0135454>] (kthread+0xdc/0xf4) [ 4.631512] [<c0135454>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c) [ 4.639239] Code: e10f7000 f10c0080 e2845004 f595f000 (e1953f9f) [ 4.645845] ---[ end trace 3bfc770b9638a54b ]--- Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America
[toc] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2016-06-08 03:30 +0200 |
| Message-ID | <rHAtH-1Qa-5@gated-at.bofh.it> |
| In reply to | #1416428 |
Hi Javier,
On 06/08/2016 01:06 AM, Javier Martinez Canillas wrote:
> Hello Yakir,
>
> On 03/17/2016 05:47 PM, Heiko Stübner wrote:
>> Split the dp core driver from exynos directory to bridge directory,
>> and rename the core driver to analogix_dp_*, rename the platform
>> code to exynos_dp.
>>
>> Beside the new analogix_dp driver would export six hooks.
>> "analogix_dp_bind()" and "analogix_dp_unbind()"
>> "analogix_dp_suspned()" and "analogix_dp_resume()"
>> "analogix_dp_detect()" and "analogix_dp_get_modes()"
>>
>> The bind/unbind symbols is used for analogix platform driver to connect
>> with analogix_dp core driver. And the detect/get_modes is used for analogix
>> platform driver to init the connector.
>>
>> They reason why connector need register in helper driver is rockchip drm
>> haven't implement the atomic API, but Exynos drm have implement it, so
>> there would need two different connector helper functions, that's why we
>> leave the connector register in helper driver.
>>
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>> ---
> Marc reported that his Exynos5250 Snow Chromebook fails to boot with v4.7-rc.
>
> I've done a git bisect and tracked down to this commit. The problem is a NULL
> pointer dereference to connector->dev in drm_mode_create(connector->dev) when
> called from exynos_dp_get_modes(). The error log is at [1].
>
> I'm trying to figure out the issue but wanted to mention in case you have any
> hints about what could be the cause. AFAICT the problem is related to the fact
> that drm_connector_init() is called in analogix_dp_bridge_attach() and the
> connector passed as argument is the one in struct analogix_dp_device *dp, but
> later exynos_dp_get_modes() calls drm_mode_create() passing the connector in
> struct exynos_dp_device *dp, which has not been previously initialized.
Agree, this should be the problem, exynos_dp->connector haven't been
initialized, driver should make exynos_dp->dp to a connector point, and
record the passing connector in exynos_dp_bridge_attach(), that should
fix this problem.
Thanks,
- Yakir
diff --git a/drivers/gpu/drm/exynos/exynos_dp.c
b/drivers/gpu/drm/exynos/exynos_dp.c
index 468498e..4c1fb3f 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -34,7 +34,7 @@
struct exynos_dp_device {
struct drm_encoder encoder;
- struct drm_connector connector;
+ struct drm_connector *connector;
struct drm_bridge *ptn_bridge;
struct drm_device *drm_dev;
struct device *dev;
@@ -70,7 +70,7 @@ static int exynos_dp_poweroff(struct
analogix_dp_plat_data *plat_data)
static int exynos_dp_get_modes(struct analogix_dp_plat_data *plat_data)
{
struct exynos_dp_device *dp = to_dp(plat_data);
- struct drm_connector *connector = &dp->connector;
+ struct drm_connector *connector = dp->connector;
struct drm_display_mode *mode;
int num_modes = 0;
@@ -103,6 +103,7 @@ static int exynos_dp_bridge_attach(struct
analogix_dp_plat_data *plat_data,
int ret;
drm_connector_register(connector);
+ dp->connector = connector;
/* Pre-empt DP connector creation if there's a bridge */
if (dp->ptn_bridge) {
> [1]
> [ 4.175676] Unable to handle kernel NULL pointer dereference at virtual address 00000210
> [ 4.182793] pgd = c0004000
> [ 4.185171] [00000210] *pgd=00000000
> [ 4.188657] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> [ 4.194164] Modules linked in:
> [ 4.196964] CPU: 1 PID: 68 Comm: kworker/1:1 Not tainted 4.7.0-rc2-next-20160606-00006-g876c3150327a #97
> [ 4.206565] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [ 4.212582] Workqueue: events hdmi_hotplug_work_func
> [ 4.217383] task: ed758000 ti: ed75c000 task.ti: ed75c000
> [ 4.222887] PC is at __mutex_lock_slowpath+0x6c/0x404
> [ 4.227913] LR is at preempt_count_add+0xa4/0x140
> [ 4.232592] pc : [<c06d21d4>] lr : [<c0139de4>] psr: 60000093
> [ 4.232592] sp : ed75dd80 ip : 00000000 fp : ee8c1180
> [ 4.244193] r10: 00000300 r9 : ed758000 r8 : 00000001
> [ 4.249262] r7 : 60000013 r6 : c0b473a8 r5 : 00000210 r4 : 0000020c
> [ 4.261955] r3 : 00000000 r2 : 00000001 r1 : ee8c2808 r0 : 00000000
> [ 4.273270] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none
> [ 4.285336] Control: 10c5387d Table: 4000406a DAC: 00000051
> [ 4.295885] Process kworker/1:1 (pid: 68, stack limit = 0xed75c210)
> [ 4.306979] Stack: (0xed75dd80 to 0xed75e000)
> [ 4.316102] dd80: ee8c1180 c03dd21c ee8f0820 00000001 0000020c 0000020c 00000000 00000000
> [ 4.329201] dda0: 00000001 dededede 00000300 c06d2578 ee8c2808 c03d00f4 ee801d80 c0b0261c
> [ 4.342362] ddc0: 00000041 ee8c2800 00000000 00000001 ee8f0810 ee8f0b65 c074d2b4 c03d35b0
> [ 4.353140] dde0: 00000000 00000300 ee8c1180 c03d81d0 c387cf74 ee8f0820 00000001 c03eaffc
> [ 4.361811] de00: 00000000 ee8f0820 00000001 c03f28bc ee8f0b65 c06d400c ee8c1180 006d40a4
> [ 4.370483] de20: 00000000 01820a11 81010300 00000100 00000064 ee8f085c ee8f0820 c0b1f894
> [ 4.379150] de40: c3872800 c0b82fac c074d2b4 c03b7550 c0b13274 c0327974 00000001 eefb8530
> [ 4.387805] de60: 00000556 00000000 c38729b0 c3872800 ee8c1180 00000001 ee8c7000 00000000
> [ 4.396467] de80: 00000556 00000300 c38729b0 00000002 ee8c1180 c03c107c ee8c7000 c3872800
> [ 4.405121] dea0: ee8c7240 c3872800 c0b82fac c03c3268 c3872ad4 00000001 c3872ae0 c03b7af0
> [ 4.413776] dec0: eefb8000 c0139804 00000000 c0136484 ee8a0000 ee8c1180 c3870b4c eefb7b00
> [ 4.422415] dee0: eefbb000 00000000 00000000 eefb7b00 ee8c1180 c012ff2c 00000008 eefb7b24
> [ 4.431045] df00: eefb7b00 ee8c1198 00000008 eefb7b24 c0b02100 ed75c000 eefb7b00 c0130178
> [ 4.439664] df20: ed75c000 ee863040 ee8c1180 00000000 ee863040 ee8c1180 c0130140 00000000
> [ 4.448274] df40: 00000000 00000000 00000000 c0135454 00000000 00000000 00000000 ee8c1180
> [ 4.456879] df60: 00000000 00000000 dead4ead ffffffff ffffffff ed75df74 ed75df74 00000000
> [ 4.465462] df80: 00000000 dead4ead ffffffff ffffffff ed75df90 ed75df90 ed75dfac ee863040
> [ 4.474028] dfa0: c0135378 00000000 00000000 c0107978 00000000 00000000 00000000 00000000
> [ 4.482597] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [ 4.491133] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
> [ 4.499643] [<c06d21d4>] (__mutex_lock_slowpath) from [<c06d2578>] (mutex_lock+0xc/0x24)
> [ 4.507102] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
> [ 4.514264] [<c06d2578>] (mutex_lock) from [<c03d00f4>] (drm_mode_object_get_reg+0x28/0x80)
> [ 4.522981] [<c03d00f4>] (drm_mode_object_get_reg) from [<c03d35b0>] (drm_mode_object_get+0x18/0x20)
> [ 4.532491] [<c03d35b0>] (drm_mode_object_get) from [<c03d81d0>] (drm_mode_create+0x3c/0x58)
> [ 4.541316] [<c03d81d0>] (drm_mode_create) from [<c03eaffc>] (exynos_dp_get_modes+0x24/0x80)
> [ 4.550160] [<c03eaffc>] (exynos_dp_get_modes) from [<c03f28bc>] (analogix_dp_get_modes+0x198/0x278)
> [ 4.559735] [<c03f28bc>] (analogix_dp_get_modes) from [<c03b7550>] (drm_helper_probe_single_connector_modes+0x114/0x554)
> [ 4.571078] [<c03b7550>] (drm_helper_probe_single_connector_modes) from [<c03c107c>] (drm_fb_helper_probe_connector_modes+0x48/0x68)
> [ 4.583513] [<c03c107c>] (drm_fb_helper_probe_connector_modes) from [<c03c3268>] (drm_fb_helper_hotplug_event+0x70/0xdc)
> [ 4.594921] [<c03c3268>] (drm_fb_helper_hotplug_event) from [<c03b7af0>] (drm_helper_hpd_irq_event+0xd4/0x160)
> [ 4.605474] [<c03b7af0>] (drm_helper_hpd_irq_event) from [<c012ff2c>] (process_one_work+0x124/0x338)
> [ 4.615144] [<c012ff2c>] (process_one_work) from [<c0130178>] (worker_thread+0x38/0x4d4)
> [ 4.623764] [<c0130178>] (worker_thread) from [<c0135454>] (kthread+0xdc/0xf4)
> [ 4.631512] [<c0135454>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c)
> [ 4.639239] Code: e10f7000 f10c0080 e2845004 f595f000 (e1953f9f)
> [ 4.645845] ---[ end trace 3bfc770b9638a54b ]---
>
> Best regards,
[toc] | [prev] | [next] | [standalone]
| From | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| Date | 2016-06-08 09:50 +0200 |
| Message-ID | <rHGps-5GS-21@gated-at.bofh.it> |
| In reply to | #1416811 |
On Wed, 8 Jun 2016 09:28:32 +0800
Yakir Yang <ykk@rock-chips.com> wrote:
> Hi Javier,
>
> On 06/08/2016 01:06 AM, Javier Martinez Canillas wrote:
> > Hello Yakir,
> >
> > On 03/17/2016 05:47 PM, Heiko Stübner wrote:
> >> Split the dp core driver from exynos directory to bridge directory,
> >> and rename the core driver to analogix_dp_*, rename the platform
> >> code to exynos_dp.
> >>
> >> Beside the new analogix_dp driver would export six hooks.
> >> "analogix_dp_bind()" and "analogix_dp_unbind()"
> >> "analogix_dp_suspned()" and "analogix_dp_resume()"
> >> "analogix_dp_detect()" and "analogix_dp_get_modes()"
> >>
> >> The bind/unbind symbols is used for analogix platform driver to connect
> >> with analogix_dp core driver. And the detect/get_modes is used for analogix
> >> platform driver to init the connector.
> >>
> >> They reason why connector need register in helper driver is rockchip drm
> >> haven't implement the atomic API, but Exynos drm have implement it, so
> >> there would need two different connector helper functions, that's why we
> >> leave the connector register in helper driver.
> >>
> >> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> >> ---
> > Marc reported that his Exynos5250 Snow Chromebook fails to boot with v4.7-rc.
> >
> > I've done a git bisect and tracked down to this commit. The problem is a NULL
> > pointer dereference to connector->dev in drm_mode_create(connector->dev) when
> > called from exynos_dp_get_modes(). The error log is at [1].
> >
> > I'm trying to figure out the issue but wanted to mention in case you have any
> > hints about what could be the cause. AFAICT the problem is related to the fact
> > that drm_connector_init() is called in analogix_dp_bridge_attach() and the
> > connector passed as argument is the one in struct analogix_dp_device *dp, but
> > later exynos_dp_get_modes() calls drm_mode_create() passing the connector in
> > struct exynos_dp_device *dp, which has not been previously initialized.
>
> Agree, this should be the problem, exynos_dp->connector haven't been
> initialized, driver should make exynos_dp->dp to a connector point, and
> record the passing connector in exynos_dp_bridge_attach(), that should
> fix this problem.
>
>
> Thanks,
> - Yakir
>
>
> diff --git a/drivers/gpu/drm/exynos/exynos_dp.c
> b/drivers/gpu/drm/exynos/exynos_dp.c
> index 468498e..4c1fb3f 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp.c
> @@ -34,7 +34,7 @@
>
> struct exynos_dp_device {
> struct drm_encoder encoder;
> - struct drm_connector connector;
> + struct drm_connector *connector;
> struct drm_bridge *ptn_bridge;
> struct drm_device *drm_dev;
> struct device *dev;
> @@ -70,7 +70,7 @@ static int exynos_dp_poweroff(struct
> analogix_dp_plat_data *plat_data)
> static int exynos_dp_get_modes(struct analogix_dp_plat_data *plat_data)
> {
> struct exynos_dp_device *dp = to_dp(plat_data);
> - struct drm_connector *connector = &dp->connector;
> + struct drm_connector *connector = dp->connector;
> struct drm_display_mode *mode;
> int num_modes = 0;
>
> @@ -103,6 +103,7 @@ static int exynos_dp_bridge_attach(struct
> analogix_dp_plat_data *plat_data,
> int ret;
>
> drm_connector_register(connector);
> + dp->connector = connector;
>
> /* Pre-empt DP connector creation if there's a bridge */
> if (dp->ptn_bridge) {
>
I've just tested this change, and in combination with Javier's DT patch,
my Snow is back to its useful state (I'm writing this email from that
very Chromebook).
Once you make this a proper patch, please add my:
Tested-by: Marc Zyngier <marc.zyngier@arm.com>
to it.
Thanks a lot to you and Javier for tracking this down!
M.
--
Jazz is not dead. It just smells funny.
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2016-06-08 13:00 +0200 |
| Message-ID | <rHJnk-7xx-7@gated-at.bofh.it> |
| In reply to | #1416996 |
Marc, Javier
On 06/08/2016 03:44 PM, Marc Zyngier wrote:
> On Wed, 8 Jun 2016 09:28:32 +0800
> Yakir Yang <ykk@rock-chips.com> wrote:
>
>> Hi Javier,
>>
>> On 06/08/2016 01:06 AM, Javier Martinez Canillas wrote:
>>> Hello Yakir,
>>>
>>> On 03/17/2016 05:47 PM, Heiko Stübner wrote:
>>>> Split the dp core driver from exynos directory to bridge directory,
>>>> and rename the core driver to analogix_dp_*, rename the platform
>>>> code to exynos_dp.
>>>>
>>>> Beside the new analogix_dp driver would export six hooks.
>>>> "analogix_dp_bind()" and "analogix_dp_unbind()"
>>>> "analogix_dp_suspned()" and "analogix_dp_resume()"
>>>> "analogix_dp_detect()" and "analogix_dp_get_modes()"
>>>>
>>>> The bind/unbind symbols is used for analogix platform driver to connect
>>>> with analogix_dp core driver. And the detect/get_modes is used for analogix
>>>> platform driver to init the connector.
>>>>
>>>> They reason why connector need register in helper driver is rockchip drm
>>>> haven't implement the atomic API, but Exynos drm have implement it, so
>>>> there would need two different connector helper functions, that's why we
>>>> leave the connector register in helper driver.
>>>>
>>>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>>>> ---
>>> Marc reported that his Exynos5250 Snow Chromebook fails to boot with v4.7-rc.
>>>
>>> I've done a git bisect and tracked down to this commit. The problem is a NULL
>>> pointer dereference to connector->dev in drm_mode_create(connector->dev) when
>>> called from exynos_dp_get_modes(). The error log is at [1].
>>>
>>> I'm trying to figure out the issue but wanted to mention in case you have any
>>> hints about what could be the cause. AFAICT the problem is related to the fact
>>> that drm_connector_init() is called in analogix_dp_bridge_attach() and the
>>> connector passed as argument is the one in struct analogix_dp_device *dp, but
>>> later exynos_dp_get_modes() calls drm_mode_create() passing the connector in
>>> struct exynos_dp_device *dp, which has not been previously initialized.
>> Agree, this should be the problem, exynos_dp->connector haven't been
>> initialized, driver should make exynos_dp->dp to a connector point, and
>> record the passing connector in exynos_dp_bridge_attach(), that should
>> fix this problem.
>>
>>
>> Thanks,
>> - Yakir
>>
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_dp.c
>> b/drivers/gpu/drm/exynos/exynos_dp.c
>> index 468498e..4c1fb3f 100644
>> --- a/drivers/gpu/drm/exynos/exynos_dp.c
>> +++ b/drivers/gpu/drm/exynos/exynos_dp.c
>> @@ -34,7 +34,7 @@
>>
>> struct exynos_dp_device {
>> struct drm_encoder encoder;
>> - struct drm_connector connector;
>> + struct drm_connector *connector;
>> struct drm_bridge *ptn_bridge;
>> struct drm_device *drm_dev;
>> struct device *dev;
>> @@ -70,7 +70,7 @@ static int exynos_dp_poweroff(struct
>> analogix_dp_plat_data *plat_data)
>> static int exynos_dp_get_modes(struct analogix_dp_plat_data *plat_data)
>> {
>> struct exynos_dp_device *dp = to_dp(plat_data);
>> - struct drm_connector *connector = &dp->connector;
>> + struct drm_connector *connector = dp->connector;
>> struct drm_display_mode *mode;
>> int num_modes = 0;
>>
>> @@ -103,6 +103,7 @@ static int exynos_dp_bridge_attach(struct
>> analogix_dp_plat_data *plat_data,
>> int ret;
>>
>> drm_connector_register(connector);
>> + dp->connector = connector;
>>
>> /* Pre-empt DP connector creation if there's a bridge */
>> if (dp->ptn_bridge) {
>>
> I've just tested this change, and in combination with Javier's DT patch,
> my Snow is back to its useful state (I'm writing this email from that
> very Chromebook).
>
> Once you make this a proper patch, please add my:
>
> Tested-by: Marc Zyngier <marc.zyngier@arm.com>
I guess Javier should be the best one to create this patch, if he have
no time, i would do it for him. thanks for your report ;)
- Yakir
> to it.
>
> Thanks a lot to you and Javier for tracking this down!
>
> M.
[toc] | [prev] | [next] | [standalone]
| From | Javier Martinez Canillas <javier@osg.samsung.com> |
|---|---|
| Date | 2016-06-08 16:30 +0200 |
| Message-ID | <rHMEy-1il-23@gated-at.bofh.it> |
| In reply to | #1417278 |
On 06/08/2016 06:53 AM, Yakir Yang wrote:
> Marc, Javier
>
> On 06/08/2016 03:44 PM, Marc Zyngier wrote:
>> On Wed, 8 Jun 2016 09:28:32 +0800
>> Yakir Yang <ykk@rock-chips.com> wrote:
>>
>>> Hi Javier,
>>>
>>> On 06/08/2016 01:06 AM, Javier Martinez Canillas wrote:
>>>> Hello Yakir,
>>>>
>>>> On 03/17/2016 05:47 PM, Heiko Stübner wrote:
>>>>> Split the dp core driver from exynos directory to bridge directory,
>>>>> and rename the core driver to analogix_dp_*, rename the platform
>>>>> code to exynos_dp.
>>>>>
>>>>> Beside the new analogix_dp driver would export six hooks.
>>>>> "analogix_dp_bind()" and "analogix_dp_unbind()"
>>>>> "analogix_dp_suspned()" and "analogix_dp_resume()"
>>>>> "analogix_dp_detect()" and "analogix_dp_get_modes()"
>>>>>
>>>>> The bind/unbind symbols is used for analogix platform driver to connect
>>>>> with analogix_dp core driver. And the detect/get_modes is used for analogix
>>>>> platform driver to init the connector.
>>>>>
>>>>> They reason why connector need register in helper driver is rockchip drm
>>>>> haven't implement the atomic API, but Exynos drm have implement it, so
>>>>> there would need two different connector helper functions, that's why we
>>>>> leave the connector register in helper driver.
>>>>>
>>>>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>>>>> ---
>>>> Marc reported that his Exynos5250 Snow Chromebook fails to boot with v4.7-rc.
>>>>
>>>> I've done a git bisect and tracked down to this commit. The problem is a NULL
>>>> pointer dereference to connector->dev in drm_mode_create(connector->dev) when
>>>> called from exynos_dp_get_modes(). The error log is at [1].
>>>>
>>>> I'm trying to figure out the issue but wanted to mention in case you have any
>>>> hints about what could be the cause. AFAICT the problem is related to the fact
>>>> that drm_connector_init() is called in analogix_dp_bridge_attach() and the
>>>> connector passed as argument is the one in struct analogix_dp_device *dp, but
>>>> later exynos_dp_get_modes() calls drm_mode_create() passing the connector in
>>>> struct exynos_dp_device *dp, which has not been previously initialized.
>>> Agree, this should be the problem, exynos_dp->connector haven't been
>>> initialized, driver should make exynos_dp->dp to a connector point, and
>>> record the passing connector in exynos_dp_bridge_attach(), that should
>>> fix this problem.
>>>
>>>
>>> Thanks,
>>> - Yakir
>>>
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_dp.c
>>> b/drivers/gpu/drm/exynos/exynos_dp.c
>>> index 468498e..4c1fb3f 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_dp.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_dp.c
>>> @@ -34,7 +34,7 @@
>>>
>>> struct exynos_dp_device {
>>> struct drm_encoder encoder;
>>> - struct drm_connector connector;
>>> + struct drm_connector *connector;
>>> struct drm_bridge *ptn_bridge;
>>> struct drm_device *drm_dev;
>>> struct device *dev;
>>> @@ -70,7 +70,7 @@ static int exynos_dp_poweroff(struct
>>> analogix_dp_plat_data *plat_data)
>>> static int exynos_dp_get_modes(struct analogix_dp_plat_data *plat_data)
>>> {
>>> struct exynos_dp_device *dp = to_dp(plat_data);
>>> - struct drm_connector *connector = &dp->connector;
>>> + struct drm_connector *connector = dp->connector;
>>> struct drm_display_mode *mode;
>>> int num_modes = 0;
>>>
>>> @@ -103,6 +103,7 @@ static int exynos_dp_bridge_attach(struct
>>> analogix_dp_plat_data *plat_data,
>>> int ret;
>>>
>>> drm_connector_register(connector);
>>> + dp->connector = connector;
>>>
>>> /* Pre-empt DP connector creation if there's a bridge */
>>> if (dp->ptn_bridge) {
>>>
>> I've just tested this change, and in combination with Javier's DT patch,
>> my Snow is back to its useful state (I'm writing this email from that
>> very Chromebook).
>>
Great. I also tested and the Snow booted but I don't have physical access
to check if the display was brought correctly, So thanks a lot for testing.
>> Once you make this a proper patch, please add my:
>>
>> Tested-by: Marc Zyngier <marc.zyngier@arm.com>
>
> I guess Javier should be the best one to create this patch, if he have no time, i would do it for him. thanks for your report ;)
>
Done.
> - Yakir
>
>> to it.
>>
>> Thanks a lot to you and Javier for tracking this down!
>>
Thanks to you for reporting all the issues!
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web