Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1216602 > unrolled thread
| Started by | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| First post | 2015-09-01 07:50 +0200 |
| Last post | 2015-09-22 10:40 +0200 |
| Articles | 20 on this page of 92 — 8 participants |
Back to article view | Back to linux.kernel
[PATCH v4 0/16] Add Analogix Core Display Port Driver Yakir Yang <ykk@rock-chips.com> - 2015-09-01 07:50 +0200
[PATCH v4 01/16] drm: exynos/dp: fix code style Yakir Yang <ykk@rock-chips.com> - 2015-09-01 07:50 +0200
Re: [PATCH v4 01/16] drm: exynos/dp: fix code style Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-09-03 02:30 +0200
Re: [PATCH v4 01/16] drm: exynos/dp: fix code style Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-09-03 07:10 +0200
Re: [PATCH v4 01/16] drm: exynos/dp: fix code style Yakir Yang <ykk@rock-chips.com> - 2015-09-03 07:40 +0200
Re: [PATCH v4 01/16] drm: exynos/dp: fix code style Joe Perches <joe@perches.com> - 2015-09-03 08:00 +0200
Re: [PATCH v4 01/16] drm: exynos/dp: fix code style Yakir Yang <ykk@rock-chips.com> - 2015-09-06 03:40 +0200
Re: [PATCH v4 01/16] drm: exynos/dp: fix code style Yakir Yang <ykk@rock-chips.com> - 2015-09-03 07:10 +0200
[PATCH v4 04/16] drm: bridge: analogix/dp: remove duplicate configuration of link rate and link count Yakir Yang <ykk@rock-chips.com> - 2015-09-01 08:00 +0200
[PATCH v4 05/16] drm: bridge: analogix/dp: dynamic parse sync_pol & interlace & colorimetry Yakir Yang <ykk@rock-chips.com> - 2015-09-01 08:00 +0200
Re: [PATCH v4 05/16] drm: bridge: analogix/dp: dynamic parse sync_pol & interlace & colorimetry Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-09-03 10:10 +0200
Re: [PATCH v4 05/16] drm: bridge: analogix/dp: dynamic parse sync_pol & interlace & colorimetry Yakir Yang <ykk@rock-chips.com> - 2015-09-06 04:10 +0200
[PATCH v4 11/16] drm: bridge: analogix/dp: add platform device type support Yakir Yang <ykk@rock-chips.com> - 2015-09-01 08:10 +0200
Re: [PATCH v4 11/16] drm: bridge: analogix/dp: add platform device type support Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-09-04 02:40 +0200
Re: [PATCH v4 11/16] drm: bridge: analogix/dp: add platform device type support Yakir Yang <ykk@rock-chips.com> - 2015-09-06 06:10 +0200
Re: [PATCH v4 11/16] drm: bridge: analogix/dp: add platform device type support Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-09-07 02:00 +0200
Re: [PATCH v4 11/16] drm: bridge: analogix/dp: add platform device type support Yakir Yang <ykk@rock-chips.com> - 2015-09-07 03:50 +0200
[PATCH v4 08/16] drm: rockchip/dp: add rockchip platform dp driver Yakir Yang <ykk@rock-chips.com> - 2015-09-01 08:10 +0200
Re: [PATCH v4 08/16] drm: rockchip/dp: add rockchip platform dp driver Heiko Stuebner <heiko@sntech.de> - 2015-09-01 16:30 +0200
Re: [PATCH v4 08/16] drm: rockchip/dp: add rockchip platform dp driver Yakir Yang <ykk@rock-chips.com> - 2015-09-01 16:50 +0200
Re: [PATCH v4 08/16] drm: rockchip/dp: add rockchip platform dp driver Heiko Stuebner <heiko@sntech.de> - 2015-09-01 23:10 +0200
Re: [PATCH v4 08/16] drm: rockchip/dp: add rockchip platform dp driver Yakir Yang <ykk@rock-chips.com> - 2015-09-02 04:00 +0200
[PATCH v4 06/16] Documentation: drm/bridge: add document for analogix_dp Yakir Yang <ykk@rock-chips.com> - 2015-09-01 08:10 +0200
[PATCH v4 09/16] drm: rockchip: add bpc and color mode setting Yakir Yang <ykk@rock-chips.com> - 2015-09-01 08:10 +0200
Re: [PATCH v4 09/16] drm: rockchip: add bpc and color mode setting Heiko Stuebner <heiko@sntech.de> - 2015-09-01 23:10 +0200
Re: [PATCH v4 09/16] drm: rockchip: add bpc and color mode setting Yakir Yang <ykk@rock-chips.com> - 2015-09-02 04:10 +0200
Re: [PATCH v4 09/16] drm: rockchip: add bpc and color mode setting Thierry Reding <treding@nvidia.com> - 2015-09-02 10:40 +0200
Re: [PATCH v4 09/16] drm: rockchip: add bpc and color mode setting Yakir Yang <ykk@rock-chips.com> - 2015-09-02 12:10 +0200
Re: [PATCH v4 09/16] drm: rockchip: add bpc and color mode setting Thierry Reding <treding@nvidia.com> - 2015-09-03 10:40 +0200
Re: [PATCH v4 09/16] drm: rockchip: add bpc and color mode setting Yakir Yang <ykk@rock-chips.com> - 2015-09-06 04:10 +0200
[PATCH v4 10/16] phy: Add driver for rockchip Display Port PHY Yakir Yang <ykk@rock-chips.com> - 2015-09-01 08:10 +0200
Re: [PATCH v4 10/16] phy: Add driver for rockchip Display Port PHY Heiko Stuebner <heiko@sntech.de> - 2015-09-01 19:00 +0200
Re: [PATCH v4 10/16] phy: Add driver for rockchip Display Port PHY Heiko Stuebner <heiko@sntech.de> - 2015-09-01 23:00 +0200
Re: [PATCH v4 10/16] phy: Add driver for rockchip Display Port PHY Yakir Yang <ykk@rock-chips.com> - 2015-09-02 03:50 +0200
Re: [PATCH v4 10/16] phy: Add driver for rockchip Display Port PHY Yakir Yang <ykk@rock-chips.com> - 2015-09-02 03:10 +0200
Re: [PATCH v4 10/16] phy: Add driver for rockchip Display Port PHY Rob Herring <robherring2@gmail.com> - 2015-09-02 15:30 +0200
Re: [PATCH v4 10/16] phy: Add driver for rockchip Display Port PHY Yakir Yang <ykk@rock-chips.com> - 2015-09-03 05:30 +0200
Re: [PATCH v4 10/16] phy: Add driver for rockchip Display Port PHY Heiko Stuebner <heiko@sntech.de> - 2015-09-03 16:00 +0200
[PATCH v4 07/16] ARM: dts: exynos/dp: remove some properties that deprecated by analogix_dp driver Yakir Yang <ykk@rock-chips.com> - 2015-09-01 08:10 +0200
Re: [PATCH v4 07/16] ARM: dts: exynos/dp: remove some properties that deprecated by analogix_dp driver Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-09-03 02:10 +0200
[PATCH v4 15/16] drm: bridge: analogix/dp: move hpd detect to connector detect function Yakir Yang <ykk@rock-chips.com> - 2015-09-01 08:20 +0200
[PATCH v4 12/16] drm: bridge: analogix/dp: add some rk3288 special registers setting Yakir Yang <ykk@rock-chips.com> - 2015-09-01 08:20 +0200
[PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Yakir Yang <ykk@rock-chips.com> - 2015-09-01 08:20 +0200
Re: [PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Rob Herring <robherring2@gmail.com> - 2015-09-02 22:20 +0200
Re: [PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Yakir Yang <ykk@rock-chips.com> - 2015-09-03 06:30 +0200
Re: [PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Thierry Reding <treding@nvidia.com> - 2015-09-03 11:10 +0200
Re: [PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-09-04 12:30 +0200
Re: [PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Thierry Reding <treding@nvidia.com> - 2015-09-07 11:10 +0200
Re: [PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Thierry Reding <treding@nvidia.com> - 2015-09-07 10:30 +0200
Re: [PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Rob Herring <robherring2@gmail.com> - 2015-09-04 23:50 +0200
Re: [PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Yakir Yang <ykk@rock-chips.com> - 2015-09-06 10:30 +0200
Re: [PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Thierry Reding <treding@nvidia.com> - 2015-09-07 10:40 +0200
Re: [PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Thierry Reding <treding@nvidia.com> - 2015-09-03 10:50 +0200
Re: [PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Rob Herring <robherring2@gmail.com> - 2015-09-04 00:00 +0200
Re: [PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Thierry Reding <treding@nvidia.com> - 2015-09-04 12:10 +0200
[PATCH v4 13/16] drm: bridge: analogix/dp: add max link rate and lane count limit for RK3288 Yakir Yang <ykk@rock-chips.com> - 2015-09-01 08:20 +0200
[PATCH v4 16/16] drm: bridge: analogix/dp: add edid modes parse in get_modes method Yakir Yang <ykk@rock-chips.com> - 2015-09-01 08:30 +0200
Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir Heiko Stuebner <heiko@sntech.de> - 2015-09-01 22:50 +0200
Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir Yakir Yang <ykk@rock-chips.com> - 2015-09-02 03:50 +0200
Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir Rob Herring <robherring2@gmail.com> - 2015-09-04 23:10 +0200
Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir Heiko Stuebner <heiko@sntech.de> - 2015-09-04 23:40 +0200
Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir Thierry Reding <treding@nvidia.com> - 2015-09-07 10:20 +0200
Re: [PATCH v4 0/16] Add Analogix Core Display Port Driver Heiko Stuebner <heiko@sntech.de> - 2015-09-02 00:20 +0200
Re: [PATCH v4 0/16] Add Analogix Core Display Port Driver Yakir Yang <ykk@rock-chips.com> - 2015-09-02 04:20 +0200
Re: [PATCH v4 0/16] Add Analogix Core Display Port Driver Thierry Reding <treding@nvidia.com> - 2015-09-21 11:20 +0200
Re: [PATCH v4 0/16] Add Analogix Core Display Port Driver Yakir Yang <ykk@rock-chips.com> - 2015-09-21 12:30 +0200
Re: [PATCH v4 0/16] Add Analogix Core Display Port Driver Thierry Reding <treding@nvidia.com> - 2015-09-21 13:30 +0200
Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir Emil Velikov <emil.l.velikov@gmail.com> - 2015-09-02 17:00 +0200
Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir Yakir Yang <ykk@rock-chips.com> - 2015-09-03 06:00 +0200
Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-09-03 03:00 +0200
Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir Yakir Yang <ykk@rock-chips.com> - 2015-09-03 07:40 +0200
Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-09-04 02:50 +0200
Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir Yakir Yang <ykk@rock-chips.com> - 2015-09-06 10:00 +0200
Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-09-07 02:30 +0200
Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir Yakir Yang <ykk@rock-chips.com> - 2015-09-07 04:30 +0200
[PATCH v5 0/17] Add Analogix Core Display Port Driver Yakir Yang <ykk@rock-chips.com> - 2015-09-22 09:30 +0200
[PATCH v5 01/17] drm: exynos: dp: convert to drm bridge mode Yakir Yang <ykk@rock-chips.com> - 2015-09-22 09:30 +0200
[PATCH v5 03/17] drm: bridge: analogix/dp: fix some obvious code style Yakir Yang <ykk@rock-chips.com> - 2015-09-22 09:40 +0200
[PATCH v5 06/17] Documentation: drm/bridge: add document for analogix_dp Yakir Yang <ykk@rock-chips.com> - 2015-09-22 09:50 +0200
[PATCH v5 07/17] ARM: dts: exynos/dp: remove some properties that deprecated by analogix_dp driver Yakir Yang <ykk@rock-chips.com> - 2015-09-22 09:50 +0200
[PATCH v5 09/17] Documentation: drm/bridge: add document for analogix_dp Yakir Yang <ykk@rock-chips.com> - 2015-09-22 09:50 +0200
[PATCH v5 04/17] drm: bridge: analogix/dp: remove duplicate configuration of link rate and link count Yakir Yang <ykk@rock-chips.com> - 2015-09-22 09:50 +0200
[PATCH v5 08/17] drm: rockchip: dp: add rockchip platform dp driver Yakir Yang <ykk@rock-chips.com> - 2015-09-22 09:50 +0200
[PATCH v5 11/17] Documentation: phy: add document for rockchip dp phy Yakir Yang <ykk@rock-chips.com> - 2015-09-22 10:00 +0200
[PATCH v5 10/17] phy: Add driver for rockchip Display Port PHY Yakir Yang <ykk@rock-chips.com> - 2015-09-22 10:00 +0200
[PATCH v5 12/17] drm: rockchip: vop: add bpc and color mode setting Yakir Yang <ykk@rock-chips.com> - 2015-09-22 10:00 +0200
[PATCH v5 13/17] drm: bridge: analogix/dp: add some rk3288 special registers setting Yakir Yang <ykk@rock-chips.com> - 2015-09-22 10:10 +0200
[PATCH v5 15/17] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Yakir Yang <ykk@rock-chips.com> - 2015-09-22 10:10 +0200
[PATCH v5 05/17] drm: bridge: analogix/dp: dynamic parse sync_pol & interlace & dynamic_range Yakir Yang <ykk@rock-chips.com> - 2015-09-22 10:10 +0200
[PATCH v5 14/17] drm: bridge: analogix/dp: add max link rate and lane count limit for RK3288 Yakir Yang <ykk@rock-chips.com> - 2015-09-22 10:10 +0200
[PATCH v5 16/17] drm: bridge: analogix/dp: move hpd detect to connector detect function Yakir Yang <ykk@rock-chips.com> - 2015-09-22 10:30 +0200
[PATCH v5 17/17] drm: bridge: analogix/dp: add edid modes parse in get_modes method Yakir Yang <ykk@rock-chips.com> - 2015-09-22 10:40 +0200
Page 4 of 5 — ← Prev page 1 2 3 [4] 5 Next page →
| From | Heiko Stuebner <heiko@sntech.de> |
|---|---|
| Date | 2015-09-04 23:40 +0200 |
| Subject | Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir |
| Message-ID | <q56Se-5Mb-5@gated-at.bofh.it> |
| In reply to | #1219322 |
Am Freitag, 4. September 2015, 16:06:02 schrieb Rob Herring:
> On Tue, Sep 1, 2015 at 3:46 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> > Am Dienstag, 1. September 2015, 13:49:58 schrieb Yakir Yang:
> >> Split the dp core driver from exynos directory to bridge
> >> directory, and rename the core driver to analogix_dp_*,
> >> leave the platform code to analogix_dp-exynos.
> >>
> >> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> >
> > [...]
> >
> >> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c
> >> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c similarity index 50%
> >> rename from drivers/gpu/drm/exynos/exynos_dp_core.c
> >> rename to drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> >> index bed0252..7d62f22 100644
> >> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> >> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> >
> > [...]
> >
> >> connector->polled = DRM_CONNECTOR_POLL_HPD;
> >>
> >> ret = drm_connector_init(dp->drm_dev, connector,
> >>
> >> - &exynos_dp_connector_funcs,
> >> + &analogix_dp_connector_funcs,
> >>
> >> DRM_MODE_CONNECTOR_eDP);
> >>
> >> if (ret) {
> >>
> >> DRM_ERROR("Failed to initialize connector with drm\n");
> >> return ret;
> >>
> >> }
> >>
> >> - drm_connector_helper_add(connector,
> >> &exynos_dp_connector_helper_funcs); +
> >> drm_connector_helper_add(connector,
> >> + &analogix_dp_connector_helper_funcs);
> >>
> >> drm_connector_register(connector);
> >
> > this should only run on exynos, as we're doing all our connector
> > registration in the core driver after all components are bound, so I
> > guess something like>
> > the following is needed:
> > if (dp->plat_data && dp->plat_data->dev_type == EXYNOS_DP)
> >
> > drm_connector_register(connector);
>
> Yuck!
>
> Surely there is a better way. From what I've seen of DRM, I have no
> doubt this is needed, but really a better solution is needed. Surely
> there can be a more generic way for the driver to determine if it
> should handle the connector or not. This seems like a common problem
> including one I have seen. What I'm working on has onchip DSI encoder
> -> ADV7533 -> HDMI. The DSI encoder can also have a direct attached
> panel. So I have to check for a bridge in the encoder driver and only
> register the connector for the panel if a bridge is not attached.
I'm also only a part-time drm meddler, so things may be inaccurate.
This conditional is not meant to prevent the registration of the dp connector,
only delay it for non-exynos drms. The connector registration does publish it
to userspace, so like i.MX we're doing that for all connectors at the same
time after all components are bound - to also make x11 happy. Exynos on the
other hand seems to register its connectors individually and I'm not sure if
they would be willing to change that.
see d3007dabeff4 ("drm/rockchip: register all connectors after bind") or
simply rockchip_drm_drv.c line 178.
and e355e7dd607b ("imx-drm: delay publishing sysfs connector entries")
Heiko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Thierry Reding <treding@nvidia.com> |
|---|---|
| Date | 2015-09-07 10:20 +0200 |
| Subject | Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir |
| Message-ID | <q5ZOG-8nj-17@gated-at.bofh.it> |
| In reply to | #1219332 |
[Multipart message — attachments visible in raw view] — view raw
On Fri, Sep 04, 2015 at 11:29:30PM +0200, Heiko Stuebner wrote:
> Am Freitag, 4. September 2015, 16:06:02 schrieb Rob Herring:
> > On Tue, Sep 1, 2015 at 3:46 PM, Heiko Stuebner <heiko@sntech.de> wrote:
> > > Am Dienstag, 1. September 2015, 13:49:58 schrieb Yakir Yang:
> > >> Split the dp core driver from exynos directory to bridge
> > >> directory, and rename the core driver to analogix_dp_*,
> > >> leave the platform code to analogix_dp-exynos.
> > >>
> > >> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> > >
> > > [...]
> > >
> > >> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c
> > >> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c similarity index 50%
> > >> rename from drivers/gpu/drm/exynos/exynos_dp_core.c
> > >> rename to drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > >> index bed0252..7d62f22 100644
> > >> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> > >> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > >
> > > [...]
> > >
> > >> connector->polled = DRM_CONNECTOR_POLL_HPD;
> > >>
> > >> ret = drm_connector_init(dp->drm_dev, connector,
> > >>
> > >> - &exynos_dp_connector_funcs,
> > >> + &analogix_dp_connector_funcs,
> > >>
> > >> DRM_MODE_CONNECTOR_eDP);
> > >>
> > >> if (ret) {
> > >>
> > >> DRM_ERROR("Failed to initialize connector with drm\n");
> > >> return ret;
> > >>
> > >> }
> > >>
> > >> - drm_connector_helper_add(connector,
> > >> &exynos_dp_connector_helper_funcs); +
> > >> drm_connector_helper_add(connector,
> > >> + &analogix_dp_connector_helper_funcs);
> > >>
> > >> drm_connector_register(connector);
> > >
> > > this should only run on exynos, as we're doing all our connector
> > > registration in the core driver after all components are bound, so I
> > > guess something like>
> > > the following is needed:
> > > if (dp->plat_data && dp->plat_data->dev_type == EXYNOS_DP)
> > >
> > > drm_connector_register(connector);
> >
> > Yuck!
> >
> > Surely there is a better way. From what I've seen of DRM, I have no
> > doubt this is needed, but really a better solution is needed. Surely
> > there can be a more generic way for the driver to determine if it
> > should handle the connector or not. This seems like a common problem
> > including one I have seen. What I'm working on has onchip DSI encoder
> > -> ADV7533 -> HDMI. The DSI encoder can also have a direct attached
> > panel. So I have to check for a bridge in the encoder driver and only
> > register the connector for the panel if a bridge is not attached.
>
> I'm also only a part-time drm meddler, so things may be inaccurate.
>
> This conditional is not meant to prevent the registration of the dp connector,
> only delay it for non-exynos drms. The connector registration does publish it
> to userspace, so like i.MX we're doing that for all connectors at the same
> time after all components are bound - to also make x11 happy. Exynos on the
> other hand seems to register its connectors individually and I'm not sure if
> they would be willing to change that.
>
> see d3007dabeff4 ("drm/rockchip: register all connectors after bind") or
> simply rockchip_drm_drv.c line 178.
>
> and e355e7dd607b ("imx-drm: delay publishing sysfs connector entries")
There really shouldn't be a reason for both drivers to behave
differently in this case. Gustavo has done a lot of work on cleaning up
the Exynos driver lately, so perhaps you should sync up with him to see
if this is something that he can integrate with his changes.
If you can't reach an agreement and Exynos must keep the exact behaviour
as it has now, then perhaps a better option would be to have the
Analogix core driver not register the connector but rather leave that up
to users of the helpers. Then you don't have to clutter the core driver
with this type of platform-specific data.
Thierry
[toc] | [prev] | [next] | [standalone]
| From | Heiko Stuebner <heiko@sntech.de> |
|---|---|
| Date | 2015-09-02 00:20 +0200 |
| Message-ID | <q424h-2Oo-1@gated-at.bofh.it> |
| In reply to | #1216602 |
Hi Yakir, Am Dienstag, 1. September 2015, 13:46:11 schrieb Yakir Yang: > The Samsung Exynos eDP controller and Rockchip RK3288 eDP controller > share the same IP, so a lot of parts can be re-used. I split the common > code into bridge directory, then rk3288 and exynos only need to keep > some platform code. Cause I can't find the exact IP name of exynos dp > controller, so I decide to name dp core driver with "analogix" which I > find in rk3288 eDP TRM ;) > > Beyond that, there are three light registers setting differents bewteen > exynos and rk3288. > 1. RK3288 have five special pll resigters which not indicata in exynos > dp controller. > 2. The address of DP_PHY_PD(dp phy power manager register) are different > between rk3288 and exynos. > 3. Rk3288 and exynos have different setting with AUX_HW_RETRY_CTL(dp debug > register). > > I have verified this series on two kinds of rockchip platform board, one > is rk3288 sdk board which connect with a 2K display port monitor, the other > is google jerry chromebook which connect with a eDP screen "cnm,n116bgeea2", > both of them works rightlly. it looks like during the rebase something did go wrong and I found some issues I mentioned in the replies to individual patches. I did prepare a branch based on mainline [0] with both the old and the new edp driver - rk3288_veyron_defconfig build both drivers into the image. While the old driver still works, I wasn't able to make the new one work yet ... the drm core does find the connector, but not that anything is connected to it. I'll try to dig deeper tomorrow, but maybe you'll see anything interesting before then. Heiko [0] https://github.com/mmind/linux-rockchip/tree/tmp/edp-with-veyron -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2015-09-02 04:20 +0200 |
| Message-ID | <q45Ox-8fT-3@gated-at.bofh.it> |
| In reply to | #1217116 |
Hi Heiko, 在 09/02/2015 05:47 AM, Heiko Stuebner 写道: > Hi Yakir, > > Am Dienstag, 1. September 2015, 13:46:11 schrieb Yakir Yang: >> The Samsung Exynos eDP controller and Rockchip RK3288 eDP controller >> share the same IP, so a lot of parts can be re-used. I split the common >> code into bridge directory, then rk3288 and exynos only need to keep >> some platform code. Cause I can't find the exact IP name of exynos dp >> controller, so I decide to name dp core driver with "analogix" which I >> find in rk3288 eDP TRM ;) >> >> Beyond that, there are three light registers setting differents bewteen >> exynos and rk3288. >> 1. RK3288 have five special pll resigters which not indicata in exynos >> dp controller. >> 2. The address of DP_PHY_PD(dp phy power manager register) are different >> between rk3288 and exynos. >> 3. Rk3288 and exynos have different setting with AUX_HW_RETRY_CTL(dp debug >> register). >> >> I have verified this series on two kinds of rockchip platform board, one >> is rk3288 sdk board which connect with a 2K display port monitor, the other >> is google jerry chromebook which connect with a eDP screen "cnm,n116bgeea2", >> both of them works rightlly. > it looks like during the rebase something did go wrong and I found some issues > I mentioned in the replies to individual patches. > > I did prepare a branch based on mainline [0] with both the old and the new edp > driver - rk3288_veyron_defconfig build both drivers into the image. > > While the old driver still works, I wasn't able to make the new one work yet > ... the drm core does find the connector, but not that anything is connected > to it. I'll try to dig deeper tomorrow, but maybe you'll see anything > interesting before then. Many thanks for your comment and debug, I would rebase on your "edp-with-veyron" branch and fix the broken, make sure v6 would work rightly at least in your side and my side. - Yakir > > Heiko > > [0] https://github.com/mmind/linux-rockchip/tree/tmp/edp-with-veyron > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Thierry Reding <treding@nvidia.com> |
|---|---|
| Date | 2015-09-21 11:20 +0200 |
| Message-ID | <qb5qq-2kt-25@gated-at.bofh.it> |
| In reply to | #1217278 |
[Multipart message — attachments visible in raw view] — view raw
On Mon, Sep 21, 2015 at 04:45:44PM +0800, Yakir Yang wrote: > Hi Heiko, > > On 09/02/2015 10:15 AM, Yakir Yang wrote: > >Hi Heiko, > > > >在 09/02/2015 05:47 AM, Heiko Stuebner 写道: > >>Hi Yakir, > >> > >>Am Dienstag, 1. September 2015, 13:46:11 schrieb Yakir Yang: > >>> The Samsung Exynos eDP controller and Rockchip RK3288 eDP > >>>controller > >>>share the same IP, so a lot of parts can be re-used. I split the common > >>>code into bridge directory, then rk3288 and exynos only need to keep > >>>some platform code. Cause I can't find the exact IP name of exynos dp > >>>controller, so I decide to name dp core driver with "analogix" which I > >>>find in rk3288 eDP TRM ;) > >>> > >>>Beyond that, there are three light registers setting differents bewteen > >>>exynos and rk3288. > >>>1. RK3288 have five special pll resigters which not indicata in exynos > >>> dp controller. > >>>2. The address of DP_PHY_PD(dp phy power manager register) are > >>>different > >>> between rk3288 and exynos. > >>>3. Rk3288 and exynos have different setting with AUX_HW_RETRY_CTL(dp > >>>debug > >>> register). > >>> > >>>I have verified this series on two kinds of rockchip platform board, > >>>one > >>>is rk3288 sdk board which connect with a 2K display port monitor, the > >>>other > >>>is google jerry chromebook which connect with a eDP screen > >>>"cnm,n116bgeea2", > >>>both of them works rightlly. > >>it looks like during the rebase something did go wrong and I found some > >>issues > >>I mentioned in the replies to individual patches. > >> > >>I did prepare a branch based on mainline [0] with both the old and the > >>new edp > >>driver - rk3288_veyron_defconfig build both drivers into the image. > >> > >>While the old driver still works, I wasn't able to make the new one work > >>yet > >>... the drm core does find the connector, but not that anything is > >>connected > >>to it. I'll try to dig deeper tomorrow, but maybe you'll see anything > >>interesting before then. > > > >Many thanks for your comment and debug, I would rebase on your > >"edp-with-veyron" branch and fix the broken, make sure v6 would > >work rightly at least in your side and my side. > > Just like we talk off line, I guess there are two tricky questions which > make analogix_dp just crash/failed on rockchip platform: > > - One is how to reach a agreement with the common way to register > connector. There would be a conflict with Exynos & IMX & Rockchip. > On analogix_dp thread, Exynos want to register connector when that > connector is ready. > On dw_hdmi thread, IMX want to register connector when all component is > already. > So Exynos & IMX & Rockchip should reach a common way to register > connector to fix this issue. > > - The other is atomic API. > The rockchip drm haven't implemented the atomic API, but the original > exynos_dp have used the atomic API on connector helper function. That's why > analogix_dp just keep crash on your side. There's really no reason not to convert Rockchip to atomic. It will have to happen eventually anyway. That said, there's another option that would allow you to side-step both of the above problems at the same time. If you turn the common code into a helper library that should give you enough flexibility to integrate it into all existing users. For example you could leave out the connector registration and let the drivers do that. Similarly since the helpers are only hooked up at registration time you could probably find a way to share the low-level code but again leave it up to the drivers to glue it all together at registration time (drivers could wrap the low-level code with atomic or non-atomic callbacks). This option may also have the benefit of loosening the coupling between DRM drivers and the helper code for this IP, which may be handy in case the drivers diverge again in the future, or ease transitions to new API. Thierry
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2015-09-21 12:30 +0200 |
| Message-ID | <qb6w9-3Rr-7@gated-at.bofh.it> |
| In reply to | #1229139 |
Hi Thierry,
Thanks for your suggest :)
On 09/21/2015 05:15 PM, Thierry Reding wrote:
> On Mon, Sep 21, 2015 at 04:45:44PM +0800, Yakir Yang wrote:
>> Hi Heiko,
>>
>> On 09/02/2015 10:15 AM, Yakir Yang wrote:
>>> Hi Heiko,
>>>
>>> 在 09/02/2015 05:47 AM, Heiko Stuebner 写道:
>>>> Hi Yakir,
>>>>
>>>> Am Dienstag, 1. September 2015, 13:46:11 schrieb Yakir Yang:
>>>>> The Samsung Exynos eDP controller and Rockchip RK3288 eDP
>>>>> controller
>>>>> share the same IP, so a lot of parts can be re-used. I split the common
>>>>> code into bridge directory, then rk3288 and exynos only need to keep
>>>>> some platform code. Cause I can't find the exact IP name of exynos dp
>>>>> controller, so I decide to name dp core driver with "analogix" which I
>>>>> find in rk3288 eDP TRM ;)
>>>>>
>>>>> Beyond that, there are three light registers setting differents bewteen
>>>>> exynos and rk3288.
>>>>> 1. RK3288 have five special pll resigters which not indicata in exynos
>>>>> dp controller.
>>>>> 2. The address of DP_PHY_PD(dp phy power manager register) are
>>>>> different
>>>>> between rk3288 and exynos.
>>>>> 3. Rk3288 and exynos have different setting with AUX_HW_RETRY_CTL(dp
>>>>> debug
>>>>> register).
>>>>>
>>>>> I have verified this series on two kinds of rockchip platform board,
>>>>> one
>>>>> is rk3288 sdk board which connect with a 2K display port monitor, the
>>>>> other
>>>>> is google jerry chromebook which connect with a eDP screen
>>>>> "cnm,n116bgeea2",
>>>>> both of them works rightlly.
>>>> it looks like during the rebase something did go wrong and I found some
>>>> issues
>>>> I mentioned in the replies to individual patches.
>>>>
>>>> I did prepare a branch based on mainline [0] with both the old and the
>>>> new edp
>>>> driver - rk3288_veyron_defconfig build both drivers into the image.
>>>>
>>>> While the old driver still works, I wasn't able to make the new one work
>>>> yet
>>>> ... the drm core does find the connector, but not that anything is
>>>> connected
>>>> to it. I'll try to dig deeper tomorrow, but maybe you'll see anything
>>>> interesting before then.
>>> Many thanks for your comment and debug, I would rebase on your
>>> "edp-with-veyron" branch and fix the broken, make sure v6 would
>>> work rightly at least in your side and my side.
>> Just like we talk off line, I guess there are two tricky questions which
>> make analogix_dp just crash/failed on rockchip platform:
>>
>> - One is how to reach a agreement with the common way to register
>> connector. There would be a conflict with Exynos & IMX & Rockchip.
>> On analogix_dp thread, Exynos want to register connector when that
>> connector is ready.
>> On dw_hdmi thread, IMX want to register connector when all component is
>> already.
>> So Exynos & IMX & Rockchip should reach a common way to register
>> connector to fix this issue.
>>
>> - The other is atomic API.
>> The rockchip drm haven't implemented the atomic API, but the original
>> exynos_dp have used the atomic API on connector helper function. That's why
>> analogix_dp just keep crash on your side.
> There's really no reason not to convert Rockchip to atomic. It will have
> to happen eventually anyway.
Do agree on this point, and I see Tomasz Figa have done some WIP
works on implementing the atomic_commit, maybe would upstream
in further.(https://chromium-review.googlesource.com/#/c/284560/1)
> That said, there's another option that would allow you to side-step both
> of the above problems at the same time. If you turn the common code into
> a helper library that should give you enough flexibility to integrate it
> into all existing users. For example you could leave out the connector
> registration and let the drivers do that. Similarly since the helpers
> are only hooked up at registration time you could probably find a way to
> share the low-level code but again leave it up to the drivers to glue it
> all together at registration time (drivers could wrap the low-level code
> with atomic or non-atomic callbacks).
Wow, sounds good, but I'm not sure I understand this rightly. Do you
mean that I could support two kinds of callbacks in analogix_dp_core
driver, and export them out. And move the connector registration code
into the helper driver (like exynos_dp.c), so helper driver could chose to
use the atomic or non-atomic callbacks. like:
-- analogix_dp_core.c
--------------------------------------------------------------------
...
struct drm_connector_funcs analogix_dp_connector_atomic_funcs = {
.dpms = drm_atomic_helper_connector_dpms,
.fill_modes = drm_helper_probe_single_connector_modes,
.detect = analogix_dp_detect,
.destroy = analogix_dp_connector_destroy,
.reset = drm_atomic_helper_connector_reset,
.atomic_duplicate_state =
drm_atomic_helper_connector_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
EXPORT_SYMBOL(analogix_dp_connector_atomic_funcs);
struct drm_connector_funcs analogix_dp_connector_funcs = {
.dpms = drm_helper_connector_dpms,
.fill_modes = drm_helper_probe_single_connector_modes,
.detect = analogix_dp_detect,
.destroy = analogix_dp_connector_destroy,
};
EXPORT_SYMBOL(analogix_dp_connector_funcs);
struct drm_connector_helper_funcs analogix_dp_connector_helper_funcs = {
.get_modes = analogix_dp_get_modes,
.best_encoder = analogix_dp_best_encoder,
};
EXPORT_SYMBOL(analogix_dp_connector_helper_funcs);
...
-- exynos_dp
----------------------------------------------------------------------------
ret = drm_connector_init(dp->drm_dev, connector,
&analogix_dp_connector_atomic_funcs,
DRM_MODE_CONNECTOR_eDP);
if (ret) {
DRM_ERROR("Failed to initialize connector with drm\n");
return ret;
}
drm_connector_helper_add(connector,
&analogix_dp_connector_helper_funcs);
drm_connector_register(connector);
drm_mode_connector_attach_encoder(connector, encoder);
-- analogix_dp-rockchip
----------------------------------------------------------------------------
ret = drm_connector_init(dp->drm_dev, connector,
&analogix_dp_connector_funcs,
DRM_MODE_CONNECTOR_eDP);
if (ret) {
DRM_ERROR("Failed to initialize connector with drm\n");
return ret;
}
drm_connector_helper_add(connector,
&analogix_dp_connector_helper_funcs);
drm_mode_connector_attach_encoder(connector, encoder);
Are those code corresponding to your suggestion. :)
Thanks
- Yakir
> This option may also have the benefit of loosening the coupling between
> DRM drivers and the helper code for this IP, which may be handy in case
> the drivers diverge again in the future, or ease transitions to new API.
>
> Thierry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Thierry Reding <treding@nvidia.com> |
|---|---|
| Date | 2015-09-21 13:30 +0200 |
| Message-ID | <qb7sd-5bY-3@gated-at.bofh.it> |
| In reply to | #1229193 |
[Multipart message — attachments visible in raw view] — view raw
On Mon, Sep 21, 2015 at 06:27:40PM +0800, Yakir Yang wrote:
> Hi Thierry,
>
> Thanks for your suggest :)
>
> On 09/21/2015 05:15 PM, Thierry Reding wrote:
> >On Mon, Sep 21, 2015 at 04:45:44PM +0800, Yakir Yang wrote:
> >>Hi Heiko,
> >>
> >>On 09/02/2015 10:15 AM, Yakir Yang wrote:
> >>>Hi Heiko,
> >>>
> >>>在 09/02/2015 05:47 AM, Heiko Stuebner 写道:
> >>>>Hi Yakir,
> >>>>
> >>>>Am Dienstag, 1. September 2015, 13:46:11 schrieb Yakir Yang:
> >>>>> The Samsung Exynos eDP controller and Rockchip RK3288 eDP
> >>>>>controller
> >>>>>share the same IP, so a lot of parts can be re-used. I split the common
> >>>>>code into bridge directory, then rk3288 and exynos only need to keep
> >>>>>some platform code. Cause I can't find the exact IP name of exynos dp
> >>>>>controller, so I decide to name dp core driver with "analogix" which I
> >>>>>find in rk3288 eDP TRM ;)
> >>>>>
> >>>>>Beyond that, there are three light registers setting differents bewteen
> >>>>>exynos and rk3288.
> >>>>>1. RK3288 have five special pll resigters which not indicata in exynos
> >>>>> dp controller.
> >>>>>2. The address of DP_PHY_PD(dp phy power manager register) are
> >>>>>different
> >>>>> between rk3288 and exynos.
> >>>>>3. Rk3288 and exynos have different setting with AUX_HW_RETRY_CTL(dp
> >>>>>debug
> >>>>> register).
> >>>>>
> >>>>>I have verified this series on two kinds of rockchip platform board,
> >>>>>one
> >>>>>is rk3288 sdk board which connect with a 2K display port monitor, the
> >>>>>other
> >>>>>is google jerry chromebook which connect with a eDP screen
> >>>>>"cnm,n116bgeea2",
> >>>>>both of them works rightlly.
> >>>>it looks like during the rebase something did go wrong and I found some
> >>>>issues
> >>>>I mentioned in the replies to individual patches.
> >>>>
> >>>>I did prepare a branch based on mainline [0] with both the old and the
> >>>>new edp
> >>>>driver - rk3288_veyron_defconfig build both drivers into the image.
> >>>>
> >>>>While the old driver still works, I wasn't able to make the new one work
> >>>>yet
> >>>>... the drm core does find the connector, but not that anything is
> >>>>connected
> >>>>to it. I'll try to dig deeper tomorrow, but maybe you'll see anything
> >>>>interesting before then.
> >>>Many thanks for your comment and debug, I would rebase on your
> >>>"edp-with-veyron" branch and fix the broken, make sure v6 would
> >>>work rightly at least in your side and my side.
> >>Just like we talk off line, I guess there are two tricky questions which
> >>make analogix_dp just crash/failed on rockchip platform:
> >>
> >>- One is how to reach a agreement with the common way to register
> >>connector. There would be a conflict with Exynos & IMX & Rockchip.
> >> On analogix_dp thread, Exynos want to register connector when that
> >>connector is ready.
> >> On dw_hdmi thread, IMX want to register connector when all component is
> >>already.
> >> So Exynos & IMX & Rockchip should reach a common way to register
> >>connector to fix this issue.
> >>
> >>- The other is atomic API.
> >> The rockchip drm haven't implemented the atomic API, but the original
> >>exynos_dp have used the atomic API on connector helper function. That's why
> >>analogix_dp just keep crash on your side.
> >There's really no reason not to convert Rockchip to atomic. It will have
> >to happen eventually anyway.
>
> Do agree on this point, and I see Tomasz Figa have done some WIP
> works on implementing the atomic_commit, maybe would upstream
> in further.(https://chromium-review.googlesource.com/#/c/284560/1)
>
>
> >That said, there's another option that would allow you to side-step both
> >of the above problems at the same time. If you turn the common code into
> >a helper library that should give you enough flexibility to integrate it
> >into all existing users. For example you could leave out the connector
> >registration and let the drivers do that. Similarly since the helpers
> >are only hooked up at registration time you could probably find a way to
> >share the low-level code but again leave it up to the drivers to glue it
> >all together at registration time (drivers could wrap the low-level code
> >with atomic or non-atomic callbacks).
>
> Wow, sounds good, but I'm not sure I understand this rightly. Do you
> mean that I could support two kinds of callbacks in analogix_dp_core
> driver, and export them out. And move the connector registration code
> into the helper driver (like exynos_dp.c), so helper driver could chose to
> use the atomic or non-atomic callbacks. like:
>
> -- analogix_dp_core.c
> --------------------------------------------------------------------
> ...
> struct drm_connector_funcs analogix_dp_connector_atomic_funcs = {
> .dpms = drm_atomic_helper_connector_dpms,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .detect = analogix_dp_detect,
> .destroy = analogix_dp_connector_destroy,
> .reset = drm_atomic_helper_connector_reset,
> .atomic_duplicate_state =
> drm_atomic_helper_connector_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> };
> EXPORT_SYMBOL(analogix_dp_connector_atomic_funcs);
>
> struct drm_connector_funcs analogix_dp_connector_funcs = {
> .dpms = drm_helper_connector_dpms,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .detect = analogix_dp_detect,
> .destroy = analogix_dp_connector_destroy,
> };
> EXPORT_SYMBOL(analogix_dp_connector_funcs);
>
> struct drm_connector_helper_funcs analogix_dp_connector_helper_funcs = {
> .get_modes = analogix_dp_get_modes,
> .best_encoder = analogix_dp_best_encoder,
> };
> EXPORT_SYMBOL(analogix_dp_connector_helper_funcs);
> ...
>
> -- exynos_dp
> ----------------------------------------------------------------------------
> ret = drm_connector_init(dp->drm_dev, connector,
> &analogix_dp_connector_atomic_funcs,
> DRM_MODE_CONNECTOR_eDP);
> if (ret) {
> DRM_ERROR("Failed to initialize connector with drm\n");
> return ret;
> }
>
> drm_connector_helper_add(connector,
> &analogix_dp_connector_helper_funcs);
> drm_connector_register(connector);
> drm_mode_connector_attach_encoder(connector, encoder);
>
> -- analogix_dp-rockchip
> ----------------------------------------------------------------------------
> ret = drm_connector_init(dp->drm_dev, connector,
> &analogix_dp_connector_funcs,
> DRM_MODE_CONNECTOR_eDP);
> if (ret) {
> DRM_ERROR("Failed to initialize connector with drm\n");
> return ret;
> }
>
> drm_connector_helper_add(connector,
> &analogix_dp_connector_helper_funcs);
> drm_mode_connector_attach_encoder(connector, encoder);
>
>
> Are those code corresponding to your suggestion. :)
Yes, that looks about right. You could also move the implementations
into the Exynos and Rockchip drivers, respectively, if they're only used
from one place. Then you can simply export the low-level analogix_dp_*()
functions. That might give you even more flexibility, but the above
would probably work well enough.
Thierry
[toc] | [prev] | [next] | [standalone]
| From | Emil Velikov <emil.l.velikov@gmail.com> |
|---|---|
| Date | 2015-09-02 17:00 +0200 |
| Subject | Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir |
| Message-ID | <q4hG2-8ct-17@gated-at.bofh.it> |
| In reply to | #1216602 |
[Dropping the CC list] Hi Yakir Yang, On 1 September 2015 at 06:49, Yakir Yang <ykk@rock-chips.com> wrote: > Split the dp core driver from exynos directory to bridge > directory, and rename the core driver to analogix_dp_*, > leave the platform code to analogix_dp-exynos. > > Signed-off-by: Yakir Yang <ykk@rock-chips.com> > --- > Changes in v4: > - Take Rob suggest, update "analogix,hpd-gpios" to "hpd-gpios" DT propery. > - Take Jingoo suggest, rename "analogix_dp-exynos.c" file name to "exynos_dp.c" > - Take Archit suggest, create a separate folder for analogix code in bridge/ > "Take X suggest", is grammatically incorrect. You should use "suggestion(s)" or alternatively use the following approach. - Create a separate folder for analogix code in bridge/ (Archit) Cheers, Emil P.S. Why do you resend the whole series (some 10+ patches) when only a few patches have been changed ? Are all the patches changed whist missing that information (vX: rebase on top of A) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2015-09-03 06:00 +0200 |
| Subject | Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir |
| Message-ID | <q4tQR-lJ-5@gated-at.bofh.it> |
| In reply to | #1217685 |
Hi Emil, 在 09/02/2015 10:50 PM, Emil Velikov 写道: > [Dropping the CC list] Hmm...Don't understand what this means. If you can explain, that would be better, so I would not miss your suggest. :-) > Hi Yakir Yang, > > On 1 September 2015 at 06:49, Yakir Yang <ykk@rock-chips.com> wrote: >> Split the dp core driver from exynos directory to bridge >> directory, and rename the core driver to analogix_dp_*, >> leave the platform code to analogix_dp-exynos. >> >> Signed-off-by: Yakir Yang <ykk@rock-chips.com> >> --- >> Changes in v4: >> - Take Rob suggest, update "analogix,hpd-gpios" to "hpd-gpios" DT propery. >> - Take Jingoo suggest, rename "analogix_dp-exynos.c" file name to "exynos_dp.c" >> - Take Archit suggest, create a separate folder for analogix code in bridge/ >> > "Take X suggest", is grammatically incorrect. You should use > "suggestion(s)" or alternatively use the following approach. > > - Create a separate folder for analogix code in bridge/ (Archit) Thanks for your correct. I prefer to take the last one ;) > Cheers, > Emil > > P.S. Why do you resend the whole series (some 10+ patches) when only a > few patches have been changed ? Are all the patches changed whist > missing that information (vX: rebase on top of A) Yes, those 10+ patches do base on the previous changes, specially when I rebase my series on the latest linux-next branch. Besides, I don't know that how to send parts of this series, and keep the unrelated patches no send. For example if the [v4 03/16] ~ [v4 10/16] need update to v5, and [v4 11/16] ~ [v4 16/16] keep in v4. So should I only send the [v5 03/16] ~ [v5 10/16], no need to send and update [v4 11/16] ~ [v4 16/16] to v5 ? If so I think it would be difficult for guys to comment, cause they can't see the whole series in one reply list . ;) Thanks, - Yakir > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Krzysztof Kozlowski <k.kozlowski@samsung.com> |
|---|---|
| Date | 2015-09-03 03:00 +0200 |
| Subject | Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir |
| Message-ID | <q4r2F-4Ip-13@gated-at.bofh.it> |
| In reply to | #1216602 |
On 01.09.2015 14:49, Yakir Yang wrote:
> Split the dp core driver from exynos directory to bridge
> directory, and rename the core driver to analogix_dp_*,
> leave the platform code to analogix_dp-exynos.
>
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> ---
> Changes in v4:
> - Take Rob suggest, update "analogix,hpd-gpios" to "hpd-gpios" DT propery.
> - Take Jingoo suggest, rename "analogix_dp-exynos.c" file name to "exynos_dp.c"
> - Take Archit suggest, create a separate folder for analogix code in bridge/
>
> Changes in v3:
> - Take Thierry Reding suggest, move exynos's video_timing code
> to analogix_dp-exynos platform driver, add get_modes method
> to struct analogix_dp_plat_data.
> - Take Heiko suggest, rename some "samsung*" dts propery to "analogix*".
>
> Changes in v2:
> - Take Jingoo Han suggest, remove new copyright
> - Fix compiled failed dut to analogix_dp_device misspell
>
> drivers/gpu/drm/bridge/Kconfig | 2 +
> drivers/gpu/drm/bridge/Makefile | 1 +
> drivers/gpu/drm/bridge/analogix/Kconfig | 4 +
> drivers/gpu/drm/bridge/analogix/Makefile | 1 +
> .../analogix/analogix_dp_core.c} | 817 ++++++-------
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 283 +++++
> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 1265 ++++++++++++++++++++
> .../analogix/analogix_dp_reg.h} | 258 ++--
> drivers/gpu/drm/exynos/Kconfig | 3 +-
> drivers/gpu/drm/exynos/Makefile | 2 +-
> drivers/gpu/drm/exynos/exynos_dp.c | 306 +++++
> drivers/gpu/drm/exynos/exynos_dp_core.h | 282 -----
> drivers/gpu/drm/exynos/exynos_dp_reg.c | 1259 -------------------
> include/drm/bridge/analogix_dp.h | 24 +
> 14 files changed, 2357 insertions(+), 2150 deletions(-)
> create mode 100644 drivers/gpu/drm/bridge/analogix/Kconfig
> create mode 100644 drivers/gpu/drm/bridge/analogix/Makefile
> rename drivers/gpu/drm/{exynos/exynos_dp_core.c => bridge/analogix/analogix_dp_core.c} (50%)
> create mode 100644 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> create mode 100644 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> rename drivers/gpu/drm/{exynos/exynos_dp_reg.h => bridge/analogix/analogix_dp_reg.h} (64%)
> create mode 100644 drivers/gpu/drm/exynos/exynos_dp.c
> delete mode 100644 drivers/gpu/drm/exynos/exynos_dp_core.h
> delete mode 100644 drivers/gpu/drm/exynos/exynos_dp_reg.c
> create mode 100644 include/drm/bridge/analogix_dp.h
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 2de52a5..7b5b77a 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -29,4 +29,6 @@ config DRM_PARADE_PS8622
> ---help---
> Parade eDP-LVDS bridge chip driver.
>
> +source "drivers/gpu/drm/bridge/analogix/Kconfig"
> +
> endmenu
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index e2eef1c..5366c6b 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -3,3 +3,4 @@ ccflags-y := -Iinclude/drm
> obj-$(CONFIG_DRM_DW_HDMI) += dw_hdmi.o
> obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
> obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
> +obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
> diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig
> new file mode 100644
> index 0000000..5ff6551
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/analogix/Kconfig
> @@ -0,0 +1,4 @@
> +config DRM_ANALOGIX_DP
> + tristate
> + depends on DRM
> + select DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/bridge/analogix/Makefile b/drivers/gpu/drm/bridge/analogix/Makefile
> new file mode 100644
> index 0000000..9107b86
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/analogix/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp_core.o analogix_dp_reg.o
> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> similarity index 50%
> rename from drivers/gpu/drm/exynos/exynos_dp_core.c
> rename to drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index bed0252..7d62f22 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1,14 +1,14 @@
> /*
> - * Samsung SoC DP (Display Port) interface driver.
> - *
> - * Copyright (C) 2012 Samsung Electronics Co., Ltd.
> - * Author: Jingoo Han <jg1.han@samsung.com>
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms of the GNU General Public License as published by the
> - * Free Software Foundation; either version 2 of the License, or (at your
> - * option) any later version.
> - */
> +* Analogix DP (Display Port) core interface driver.
> +*
> +* Copyright (C) 2012 Samsung Electronics Co., Ltd.
> +* Author: Jingoo Han <jg1.han@samsung.com>
> +*
> +* This program is free software; you can redistribute it and/or modify it
> +* under the terms of the GNU General Public License as published by the
> +* Free Software Foundation; either version 2 of the License, or (at your
> +* option) any later version.
> +*/
>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> @@ -18,12 +18,9 @@
> #include <linux/interrupt.h>
> #include <linux/of.h>
> #include <linux/of_gpio.h>
> -#include <linux/of_graph.h>
> #include <linux/gpio.h>
> #include <linux/component.h>
> #include <linux/phy/phy.h>
> -#include <video/of_display_timing.h>
> -#include <video/of_videomode.h>
>
> #include <drm/drmP.h>
> #include <drm/drm_crtc.h>
> @@ -31,52 +28,42 @@
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_panel.h>
>
> -#include "exynos_dp_core.h"
> -#include "exynos_drm_crtc.h"
> +#include <drm/bridge/analogix_dp.h>
>
> -#define ctx_from_connector(c) container_of(c, struct exynos_dp_device, \
> - connector)
> +#include "analogix_dp_core.h"
>
> -static inline struct exynos_drm_crtc *dp_to_crtc(struct exynos_dp_device *dp)
> -{
> - return to_exynos_crtc(dp->encoder.crtc);
> -}
> -
> -static inline struct exynos_dp_device *encoder_to_dp(
> - struct drm_encoder *e)
> -{
> - return container_of(e, struct exynos_dp_device, encoder);
> -}
> +#define connector_to_dp(c) \
> + container_of(c, struct analogix_dp_device, connector)
>
> struct bridge_init {
> struct i2c_client *client;
> struct device_node *node;
> };
>
> -static void exynos_dp_init_dp(struct exynos_dp_device *dp)
> +static void analogix_dp_init_dp(struct analogix_dp_device *dp)
> {
> - exynos_dp_reset(dp);
> + analogix_dp_reset(dp);
>
> - exynos_dp_swreset(dp);
> + analogix_dp_swreset(dp);
>
> - exynos_dp_init_analog_param(dp);
> - exynos_dp_init_interrupt(dp);
> + analogix_dp_init_analog_param(dp);
> + analogix_dp_init_interrupt(dp);
>
> /* SW defined function Normal operation */
> - exynos_dp_enable_sw_function(dp);
> + analogix_dp_enable_sw_function(dp);
>
> - exynos_dp_config_interrupt(dp);
> - exynos_dp_init_analog_func(dp);
> + analogix_dp_config_interrupt(dp);
> + analogix_dp_init_analog_func(dp);
>
> - exynos_dp_init_hpd(dp);
> - exynos_dp_init_aux(dp);
> + analogix_dp_init_hpd(dp);
> + analogix_dp_init_aux(dp);
> }
>
> -static int exynos_dp_detect_hpd(struct exynos_dp_device *dp)
> +static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
> {
> int timeout_loop = 0;
>
> - while (exynos_dp_get_plug_in_status(dp) != 0) {
> + while (analogix_dp_get_plug_in_status(dp) != 0) {
> timeout_loop++;
> if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
> dev_err(dp->dev, "failed to get hpd plug status\n");
> @@ -88,7 +75,7 @@ static int exynos_dp_detect_hpd(struct exynos_dp_device *dp)
> return 0;
> }
>
> -static unsigned char exynos_dp_calc_edid_check_sum(unsigned char *edid_data)
> +static unsigned char analogix_dp_calc_edid_check_sum(unsigned char *edid_data)
> {
> int i;
> unsigned char sum = 0;
> @@ -99,7 +86,7 @@ static unsigned char exynos_dp_calc_edid_check_sum(unsigned char *edid_data)
> return sum;
> }
>
> -static int exynos_dp_read_edid(struct exynos_dp_device *dp)
> +static int analogix_dp_read_edid(struct analogix_dp_device *dp)
> {
> unsigned char edid[EDID_BLOCK_LENGTH * 2];
> unsigned int extend_block = 0;
> @@ -114,9 +101,9 @@ static int exynos_dp_read_edid(struct exynos_dp_device *dp)
> */
>
> /* Read Extension Flag, Number of 128-byte EDID extension blocks */
> - retval = exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
> - EDID_EXTENSION_FLAG,
> - &extend_block);
> + retval = analogix_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
> + EDID_EXTENSION_FLAG,
> + &extend_block);
Hi,
What was the point of patch 1 (checkpatch -f --subjective) if you
have to re-align the parameters again? It's meaningless.
I could understand that idea if this patch touched only first
line - name of the function. This would result in small
and effective diff, like:
- retval = exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
+ retval = SAME_LEGNTH____byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
EDID_EXTENSION_FLAG,
But this is not the case. The reviewer have to look at 6 lines
of changes instead of two. You fixed the alignment just for two
patches and then you re-written it. It's meaningless.
Nope. Please drop all changes from patch 1 which:
(all conditions apply):
1. change alignment,
2. are immediately changed in next patches (e.g. like here),
and fix the alignment while renaming the function.
This is actually another point for not accepting commits where
the reason is "checkpatch told me".
> if (retval)
> return retval;
>
> @@ -124,7 +111,7 @@ static int exynos_dp_read_edid(struct exynos_dp_device *dp)
> dev_dbg(dp->dev, "EDID data includes a single extension!\n");
>
> /* Read EDID data */
> - retval = exynos_dp_read_bytes_from_i2c(
> + retval = analogix_dp_read_bytes_from_i2c(
> dp, I2C_EDID_DEVICE_ADDR,
> EDID_HEADER_PATTERN,
> EDID_BLOCK_LENGTH,
> @@ -133,14 +120,14 @@ static int exynos_dp_read_edid(struct exynos_dp_device *dp)
(...)
> @@ -871,218 +862,204 @@ static irqreturn_t exynos_dp_irq_handler(int irq, void *arg)
> return IRQ_HANDLED;
> }
>
> -static void exynos_dp_hotplug(struct work_struct *work)
> +static void analogix_dp_hotplug(struct work_struct *work)
> {
> - struct exynos_dp_device *dp;
> + struct analogix_dp_device *dp;
>
> - dp = container_of(work, struct exynos_dp_device, hotplug_work);
> + dp = container_of(work, struct analogix_dp_device, hotplug_work);
>
> if (dp->drm_dev)
> drm_helper_hpd_irq_event(dp->drm_dev);
> }
>
> -static void exynos_dp_commit(struct drm_encoder *encoder)
> +static void analogix_dp_commit(struct analogix_dp_device *dp)
> {
> - struct exynos_dp_device *dp = encoder_to_dp(encoder);
> int ret;
>
> /* Keep the panel disabled while we configure video */
> - if (dp->panel) {
> - if (drm_panel_disable(dp->panel))
> + if (dp->plat_data && dp->plat_data->panel) {
> + if (drm_panel_disable(dp->plat_data->panel))
> DRM_ERROR("failed to disable the panel\n");
> }
>
> - ret = exynos_dp_detect_hpd(dp);
> + ret = analogix_dp_detect_hpd(dp);
> if (ret) {
> /* Cable has been disconnected, we're done */
> return;
> }
>
> - ret = exynos_dp_handle_edid(dp);
> + ret = analogix_dp_handle_edid(dp);
> if (ret) {
> dev_err(dp->dev, "unable to handle edid\n");
> return;
> }
>
> - ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
> - dp->video_info->link_rate);
> + ret = analogix_dp_set_link_train(dp, dp->video_info->lane_count,
> + dp->video_info->link_rate);
> if (ret) {
> dev_err(dp->dev, "unable to do link train\n");
> return;
> }
>
> - exynos_dp_enable_scramble(dp, 1);
> - exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
> - exynos_dp_enable_enhanced_mode(dp, 1);
> + analogix_dp_enable_scramble(dp, 1);
> + analogix_dp_enable_rx_to_enhanced_mode(dp, 1);
> + analogix_dp_enable_enhanced_mode(dp, 1);
>
> - exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
> - exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
> + analogix_dp_set_lane_count(dp, dp->video_info->lane_count);
> + analogix_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
>
> - exynos_dp_init_video(dp);
> - ret = exynos_dp_config_video(dp);
> + analogix_dp_init_video(dp);
> + ret = analogix_dp_config_video(dp);
> if (ret)
> dev_err(dp->dev, "unable to config video\n");
>
> /* Safe to enable the panel now */
> - if (dp->panel) {
> - if (drm_panel_enable(dp->panel))
> + if (dp->plat_data && dp->plat_data->panel) {
> + if (drm_panel_enable(dp->plat_data->panel))
> DRM_ERROR("failed to enable the panel\n");
> }
>
> /* Enable video */
> - exynos_dp_start_video(dp);
> + analogix_dp_start_video(dp);
> }
>
> static enum drm_connector_status
> -exynos_dp_detect(struct drm_connector *connector, bool force)
> +analogix_dp_detect(struct drm_connector *connector, bool force)
> {
> return connector_status_connected;
> }
>
> -static void exynos_dp_connector_destroy(struct drm_connector *connector)
> +static void analogix_dp_connector_destroy(struct drm_connector *connector)
> {
> drm_connector_unregister(connector);
> drm_connector_cleanup(connector);
> }
>
> -static struct drm_connector_funcs exynos_dp_connector_funcs = {
> +static struct drm_connector_funcs analogix_dp_connector_funcs = {
> .dpms = drm_atomic_helper_connector_dpms,
> .fill_modes = drm_helper_probe_single_connector_modes,
> - .detect = exynos_dp_detect,
> - .destroy = exynos_dp_connector_destroy,
> + .detect = analogix_dp_detect,
> + .destroy = analogix_dp_connector_destroy,
> .reset = drm_atomic_helper_connector_reset,
> .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> };
>
> -static int exynos_dp_get_modes(struct drm_connector *connector)
> +static int analogix_dp_get_modes(struct drm_connector *connector)
> {
> - struct exynos_dp_device *dp = ctx_from_connector(connector);
> - struct drm_display_mode *mode;
> -
> - if (dp->panel)
> - return drm_panel_get_modes(dp->panel);
> + struct analogix_dp_device *dp = connector_to_dp(connector);
> + struct analogix_dp_plat_data *plat_data = dp->plat_data;
> + int num_modes = 0;
>
> - mode = drm_mode_create(connector->dev);
> - if (!mode) {
> - DRM_ERROR("failed to create a new display mode.\n");
> - return 0;
> - }
> -
> - drm_display_mode_from_videomode(&dp->priv.vm, mode);
> - mode->width_mm = dp->priv.width_mm;
> - mode->height_mm = dp->priv.height_mm;
> - connector->display_info.width_mm = mode->width_mm;
> - connector->display_info.height_mm = mode->height_mm;
> + if (plat_data && plat_data->panel)
> + num_modes += drm_panel_get_modes(plat_data->panel);
>
> - mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> - drm_mode_set_name(mode);
> - drm_mode_probed_add(connector, mode);
> + if (plat_data && plat_data->get_modes)
> + num_modes += plat_data->get_modes(plat_data, connector);
>
> - return 1;
> + return num_modes;
> }
>
> static struct drm_encoder *
> -exynos_dp_best_encoder(struct drm_connector *connector)
> +analogix_dp_best_encoder(struct drm_connector *connector)
> {
> - struct exynos_dp_device *dp = ctx_from_connector(connector);
> + struct analogix_dp_device *dp = connector_to_dp(connector);
>
> - return &dp->encoder;
> + return dp->encoder;
> }
>
> -static struct drm_connector_helper_funcs exynos_dp_connector_helper_funcs = {
> - .get_modes = exynos_dp_get_modes,
> - .best_encoder = exynos_dp_best_encoder,
> +static struct drm_connector_helper_funcs analogix_dp_connector_helper_funcs = {
> + .get_modes = analogix_dp_get_modes,
> + .best_encoder = analogix_dp_best_encoder,
> };
>
> -/* returns the number of bridges attached */
> -static int exynos_drm_attach_lcd_bridge(struct exynos_dp_device *dp,
> - struct drm_encoder *encoder)
> -{
> - int ret;
> -
> - encoder->bridge->next = dp->ptn_bridge;
> - dp->ptn_bridge->encoder = encoder;
> - ret = drm_bridge_attach(encoder->dev, dp->ptn_bridge);
> - if (ret) {
> - DRM_ERROR("Failed to attach bridge to drm\n");
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> -static int exynos_dp_bridge_attach(struct drm_bridge *bridge)
> +static int analogix_dp_bridge_attach(struct drm_bridge *bridge)
> {
> - struct exynos_dp_device *dp = bridge->driver_private;
> - struct drm_encoder *encoder = &dp->encoder;
> + struct analogix_dp_device *dp = bridge->driver_private;
> + struct drm_encoder *encoder = dp->encoder;
> struct drm_connector *connector = &dp->connector;
> int ret;
>
> - /* Pre-empt DP connector creation if there's a bridge */
> - if (dp->ptn_bridge) {
> - ret = exynos_drm_attach_lcd_bridge(dp, encoder);
> - if (!ret)
> - return 0;
> + if (!bridge->encoder) {
> + DRM_ERROR("Parent encoder object not found");
> + return -ENODEV;
> }
>
> + encoder->bridge = bridge;
> +
> connector->polled = DRM_CONNECTOR_POLL_HPD;
>
> ret = drm_connector_init(dp->drm_dev, connector,
> - &exynos_dp_connector_funcs,
> + &analogix_dp_connector_funcs,
> DRM_MODE_CONNECTOR_eDP);
> if (ret) {
> DRM_ERROR("Failed to initialize connector with drm\n");
> return ret;
> }
>
> - drm_connector_helper_add(connector, &exynos_dp_connector_helper_funcs);
> + drm_connector_helper_add(connector,
> + &analogix_dp_connector_helper_funcs);
> drm_connector_register(connector);
> drm_mode_connector_attach_encoder(connector, encoder);
>
> - if (dp->panel)
> - ret = drm_panel_attach(dp->panel, &dp->connector);
> + if (dp->plat_data && dp->plat_data->panel) {
> + ret = drm_panel_attach(dp->plat_data->panel, &dp->connector);
> + if (ret) {
> + DRM_ERROR("Failed to attach panel\n");
> + return ret;
> + }
> + }
> +
> + /*
> + * This should be the end of attach function, caused
> + * we should ensure dp bridge could attach first.
> + */
> + if (dp->plat_data && dp->plat_data->attach) {
> + ret = dp->plat_data->attach(dp->plat_data, bridge);
> + if (ret) {
> + DRM_ERROR("Failed at platform attch func\n");
Two new error paths appeared here and above. Don't you have to
cleanup something? I don't know, just wondering...
(...)
> @@ -1345,22 +1286,9 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
> disable_irq(dp->irq);
>
> dp->drm_dev = drm_dev;
> + dp->encoder = encoder;
>
> - pipe = exynos_drm_crtc_get_pipe_from_type(drm_dev,
> - EXYNOS_DISPLAY_TYPE_LCD);
> - if (pipe < 0)
> - return pipe;
> -
> - encoder->possible_crtcs = 1 << pipe;
> -
> - DRM_DEBUG_KMS("possible_crtcs = 0x%x\n", encoder->possible_crtcs);
> -
> - drm_encoder_init(drm_dev, encoder, &exynos_dp_encoder_funcs,
> - DRM_MODE_ENCODER_TMDS);
> -
> - drm_encoder_helper_add(encoder, &exynos_dp_encoder_helper_funcs);
> -
> - ret = exynos_dp_create_connector(encoder);
> + ret = analogix_dp_create_bridge(drm_dev, dp);
> if (ret) {
> DRM_ERROR("failed to create connector ret = %d\n", ret);
> drm_encoder_cleanup(encoder);
> @@ -1369,103 +1297,36 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(analogix_dp_bind);
Ooo, a new export symbol. I suppose it is necessary for the split
but the commit message does not mention exporting
anything nor the reason for doing it. Could you extend the commit
message describing more tha changes here and rationale for it?
(...)
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> new file mode 100644
> index 0000000..b879d8c
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -0,0 +1,1265 @@
> +/*
> + * Analogix DP (Display port) core register interface driver.
> + *
> + * Copyright (C) 2012 Samsung Electronics Co., Ltd.
> + * Author: Jingoo Han <jg1.han@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +
> +#include "analogix_dp_core.h"
> +#include "analogix_dp_reg.h"
> +
> +#define COMMON_INT_MASK_1 0
> +#define COMMON_INT_MASK_2 0
> +#define COMMON_INT_MASK_3 0
> +#define COMMON_INT_MASK_4 (HOTPLUG_CHG | HPD_LOST | PLUG)
> +#define INT_STA_MASK INT_HPD
> +
> +void analogix_dp_enable_video_mute(struct analogix_dp_device *dp, bool enable)
> +{
> + u32 reg;
> +
> + if (enable) {
> + reg = readl(dp->reg_base + ANALOGIX_DP_VIDEO_CTL_1);
> + reg |= HDCP_VIDEO_MUTE;
> + writel(reg, dp->reg_base + ANALOGIX_DP_VIDEO_CTL_1);
> + } else {
> + reg = readl(dp->reg_base + ANALOGIX_DP_VIDEO_CTL_1);
> + reg &= ~HDCP_VIDEO_MUTE;
> + writel(reg, dp->reg_base + ANALOGIX_DP_VIDEO_CTL_1);
> + }
> +}
> +
> +void analogix_dp_stop_video(struct analogix_dp_device *dp)
> +{
> + u32 reg;
> +
> + reg = readl(dp->reg_base + ANALOGIX_DP_VIDEO_CTL_1);
> + reg &= ~VIDEO_EN;
> + writel(reg, dp->reg_base + ANALOGIX_DP_VIDEO_CTL_1);
> +}
> +
> +void analogix_dp_lane_swap(struct analogix_dp_device *dp, bool enable)
> +{
> + u32 reg;
> +
> + if (enable)
> + reg = LANE3_MAP_LOGIC_LANE_0 | LANE2_MAP_LOGIC_LANE_1 |
> + LANE1_MAP_LOGIC_LANE_2 | LANE0_MAP_LOGIC_LANE_3;
> + else
> + reg = LANE3_MAP_LOGIC_LANE_3 | LANE2_MAP_LOGIC_LANE_2 |
> + LANE1_MAP_LOGIC_LANE_1 | LANE0_MAP_LOGIC_LANE_0;
Here and in few other places with new code: no alignment?
You spent some time fixing these in patch 1, so I would assume
that new code already comes after "checkpatch -f --subjective".
That's actually the only valid reason for using checkpatch - new code.
However my mail reader could corrupt the output so maybe this is aligned?
Code seems fine after looking at it but review was rather shallow.
I don't know the Exynos DP driver.
Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2015-09-03 07:40 +0200 |
| Subject | Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir |
| Message-ID | <q4vpE-2GW-7@gated-at.bofh.it> |
| In reply to | #1217978 |
Hi Krzysztof,
在 09/03/2015 08:58 AM, Krzysztof Kozlowski 写道:
> On 01.09.2015 14:49, Yakir Yang wrote:
>> Split the dp core driver from exynos directory to bridge
>> directory, and rename the core driver to analogix_dp_*,
>> leave the platform code to analogix_dp-exynos.
>>
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>> ---
>> Changes in v4:
>> - Take Rob suggest, update "analogix,hpd-gpios" to "hpd-gpios" DT propery.
>> - Take Jingoo suggest, rename "analogix_dp-exynos.c" file name to "exynos_dp.c"
>> - Take Archit suggest, create a separate folder for analogix code in bridge/
>>
>> Changes in v3:
>> - Take Thierry Reding suggest, move exynos's video_timing code
>> to analogix_dp-exynos platform driver, add get_modes method
>> to struct analogix_dp_plat_data.
>> - Take Heiko suggest, rename some "samsung*" dts propery to "analogix*".
>>
>> Changes in v2:
>> - Take Jingoo Han suggest, remove new copyright
>> - Fix compiled failed dut to analogix_dp_device misspell
>>
>> drivers/gpu/drm/bridge/Kconfig | 2 +
>> drivers/gpu/drm/bridge/Makefile | 1 +
>> drivers/gpu/drm/bridge/analogix/Kconfig | 4 +
>> drivers/gpu/drm/bridge/analogix/Makefile | 1 +
>> .../analogix/analogix_dp_core.c} | 817 ++++++-------
>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 283 +++++
>> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 1265 ++++++++++++++++++++
>> .../analogix/analogix_dp_reg.h} | 258 ++--
>> drivers/gpu/drm/exynos/Kconfig | 3 +-
>> drivers/gpu/drm/exynos/Makefile | 2 +-
>> drivers/gpu/drm/exynos/exynos_dp.c | 306 +++++
>> drivers/gpu/drm/exynos/exynos_dp_core.h | 282 -----
>> drivers/gpu/drm/exynos/exynos_dp_reg.c | 1259 -------------------
>> include/drm/bridge/analogix_dp.h | 24 +
>> 14 files changed, 2357 insertions(+), 2150 deletions(-)
>> create mode 100644 drivers/gpu/drm/bridge/analogix/Kconfig
>> create mode 100644 drivers/gpu/drm/bridge/analogix/Makefile
>> rename drivers/gpu/drm/{exynos/exynos_dp_core.c => bridge/analogix/analogix_dp_core.c} (50%)
>> create mode 100644 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> create mode 100644 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> rename drivers/gpu/drm/{exynos/exynos_dp_reg.h => bridge/analogix/analogix_dp_reg.h} (64%)
>> create mode 100644 drivers/gpu/drm/exynos/exynos_dp.c
>> delete mode 100644 drivers/gpu/drm/exynos/exynos_dp_core.h
>> delete mode 100644 drivers/gpu/drm/exynos/exynos_dp_reg.c
>> create mode 100644 include/drm/bridge/analogix_dp.h
>>
>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
>> index 2de52a5..7b5b77a 100644
>> --- a/drivers/gpu/drm/bridge/Kconfig
>> +++ b/drivers/gpu/drm/bridge/Kconfig
>> @@ -29,4 +29,6 @@ config DRM_PARADE_PS8622
>> ---help---
>> Parade eDP-LVDS bridge chip driver.
>>
>> +source "drivers/gpu/drm/bridge/analogix/Kconfig"
>> +
>> endmenu
>> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
>> index e2eef1c..5366c6b 100644
>> --- a/drivers/gpu/drm/bridge/Makefile
>> +++ b/drivers/gpu/drm/bridge/Makefile
>> @@ -3,3 +3,4 @@ ccflags-y := -Iinclude/drm
>> obj-$(CONFIG_DRM_DW_HDMI) += dw_hdmi.o
>> obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
>> obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
>> +obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>> diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig
>> new file mode 100644
>> index 0000000..5ff6551
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/analogix/Kconfig
>> @@ -0,0 +1,4 @@
>> +config DRM_ANALOGIX_DP
>> + tristate
>> + depends on DRM
>> + select DRM_KMS_HELPER
>> diff --git a/drivers/gpu/drm/bridge/analogix/Makefile b/drivers/gpu/drm/bridge/analogix/Makefile
>> new file mode 100644
>> index 0000000..9107b86
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/analogix/Makefile
>> @@ -0,0 +1 @@
>> +obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp_core.o analogix_dp_reg.o
>> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> similarity index 50%
>> rename from drivers/gpu/drm/exynos/exynos_dp_core.c
>> rename to drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index bed0252..7d62f22 100644
>> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -1,14 +1,14 @@
>> /*
>> - * Samsung SoC DP (Display Port) interface driver.
>> - *
>> - * Copyright (C) 2012 Samsung Electronics Co., Ltd.
>> - * Author: Jingoo Han <jg1.han@samsung.com>
>> - *
>> - * This program is free software; you can redistribute it and/or modify it
>> - * under the terms of the GNU General Public License as published by the
>> - * Free Software Foundation; either version 2 of the License, or (at your
>> - * option) any later version.
>> - */
>> +* Analogix DP (Display Port) core interface driver.
>> +*
>> +* Copyright (C) 2012 Samsung Electronics Co., Ltd.
>> +* Author: Jingoo Han <jg1.han@samsung.com>
>> +*
>> +* This program is free software; you can redistribute it and/or modify it
>> +* under the terms of the GNU General Public License as published by the
>> +* Free Software Foundation; either version 2 of the License, or (at your
>> +* option) any later version.
>> +*/
>>
>> #include <linux/module.h>
>> #include <linux/platform_device.h>
>> @@ -18,12 +18,9 @@
>> #include <linux/interrupt.h>
>> #include <linux/of.h>
>> #include <linux/of_gpio.h>
>> -#include <linux/of_graph.h>
>> #include <linux/gpio.h>
>> #include <linux/component.h>
>> #include <linux/phy/phy.h>
>> -#include <video/of_display_timing.h>
>> -#include <video/of_videomode.h>
>>
>> #include <drm/drmP.h>
>> #include <drm/drm_crtc.h>
>> @@ -31,52 +28,42 @@
>> #include <drm/drm_atomic_helper.h>
>> #include <drm/drm_panel.h>
>>
>> -#include "exynos_dp_core.h"
>> -#include "exynos_drm_crtc.h"
>> +#include <drm/bridge/analogix_dp.h>
>>
>> -#define ctx_from_connector(c) container_of(c, struct exynos_dp_device, \
>> - connector)
>> +#include "analogix_dp_core.h"
>>
>> -static inline struct exynos_drm_crtc *dp_to_crtc(struct exynos_dp_device *dp)
>> -{
>> - return to_exynos_crtc(dp->encoder.crtc);
>> -}
>> -
>> -static inline struct exynos_dp_device *encoder_to_dp(
>> - struct drm_encoder *e)
>> -{
>> - return container_of(e, struct exynos_dp_device, encoder);
>> -}
>> +#define connector_to_dp(c) \
>> + container_of(c, struct analogix_dp_device, connector)
>>
>> struct bridge_init {
>> struct i2c_client *client;
>> struct device_node *node;
>> };
>>
>> -static void exynos_dp_init_dp(struct exynos_dp_device *dp)
>> +static void analogix_dp_init_dp(struct analogix_dp_device *dp)
>> {
>> - exynos_dp_reset(dp);
>> + analogix_dp_reset(dp);
>>
>> - exynos_dp_swreset(dp);
>> + analogix_dp_swreset(dp);
>>
>> - exynos_dp_init_analog_param(dp);
>> - exynos_dp_init_interrupt(dp);
>> + analogix_dp_init_analog_param(dp);
>> + analogix_dp_init_interrupt(dp);
>>
>> /* SW defined function Normal operation */
>> - exynos_dp_enable_sw_function(dp);
>> + analogix_dp_enable_sw_function(dp);
>>
>> - exynos_dp_config_interrupt(dp);
>> - exynos_dp_init_analog_func(dp);
>> + analogix_dp_config_interrupt(dp);
>> + analogix_dp_init_analog_func(dp);
>>
>> - exynos_dp_init_hpd(dp);
>> - exynos_dp_init_aux(dp);
>> + analogix_dp_init_hpd(dp);
>> + analogix_dp_init_aux(dp);
>> }
>>
>> -static int exynos_dp_detect_hpd(struct exynos_dp_device *dp)
>> +static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
>> {
>> int timeout_loop = 0;
>>
>> - while (exynos_dp_get_plug_in_status(dp) != 0) {
>> + while (analogix_dp_get_plug_in_status(dp) != 0) {
>> timeout_loop++;
>> if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
>> dev_err(dp->dev, "failed to get hpd plug status\n");
>> @@ -88,7 +75,7 @@ static int exynos_dp_detect_hpd(struct exynos_dp_device *dp)
>> return 0;
>> }
>>
>> -static unsigned char exynos_dp_calc_edid_check_sum(unsigned char *edid_data)
>> +static unsigned char analogix_dp_calc_edid_check_sum(unsigned char *edid_data)
>> {
>> int i;
>> unsigned char sum = 0;
>> @@ -99,7 +86,7 @@ static unsigned char exynos_dp_calc_edid_check_sum(unsigned char *edid_data)
>> return sum;
>> }
>>
>> -static int exynos_dp_read_edid(struct exynos_dp_device *dp)
>> +static int analogix_dp_read_edid(struct analogix_dp_device *dp)
>> {
>> unsigned char edid[EDID_BLOCK_LENGTH * 2];
>> unsigned int extend_block = 0;
>> @@ -114,9 +101,9 @@ static int exynos_dp_read_edid(struct exynos_dp_device *dp)
>> */
>>
>> /* Read Extension Flag, Number of 128-byte EDID extension blocks */
>> - retval = exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
>> - EDID_EXTENSION_FLAG,
>> - &extend_block);
>> + retval = analogix_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
>> + EDID_EXTENSION_FLAG,
>> + &extend_block);
> Hi,
>
> What was the point of patch 1 (checkpatch -f --subjective) if you
> have to re-align the parameters again? It's meaningless.
>
> I could understand that idea if this patch touched only first
> line - name of the function. This would result in small
> and effective diff, like:
>
> - retval = exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
> + retval = SAME_LEGNTH____byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
> EDID_EXTENSION_FLAG,
>
> But this is not the case. The reviewer have to look at 6 lines
> of changes instead of two. You fixed the alignment just for two
> patches and then you re-written it. It's meaningless.
Yes, this is the place that crazy me too. After rename "exynos"
to "analogix", there would bring some "over-80-char" problem :(
> Nope. Please drop all changes from patch 1 which:
> (all conditions apply):
> 1. change alignment,
> 2. are immediately changed in next patches (e.g. like here),
>
> and fix the alignment while renaming the function.
>
> This is actually another point for not accepting commits where
> the reason is "checkpatch told me".
Okay, so there would be two patches to fix the alignment, done.
Thanks for your careful remind.
>
>
>> if (retval)
>> return retval;
>>
>> @@ -124,7 +111,7 @@ static int exynos_dp_read_edid(struct exynos_dp_device *dp)
>> dev_dbg(dp->dev, "EDID data includes a single extension!\n");
>>
>> /* Read EDID data */
>> - retval = exynos_dp_read_bytes_from_i2c(
>> + retval = analogix_dp_read_bytes_from_i2c(
>> dp, I2C_EDID_DEVICE_ADDR,
>> EDID_HEADER_PATTERN,
>> EDID_BLOCK_LENGTH,
>> @@ -133,14 +120,14 @@ static int exynos_dp_read_edid(struct exynos_dp_device *dp)
> (...)
>
>> @@ -871,218 +862,204 @@ static irqreturn_t exynos_dp_irq_handler(int irq, void *arg)
>> return IRQ_HANDLED;
>> }
>>
>> -static void exynos_dp_hotplug(struct work_struct *work)
>> +static void analogix_dp_hotplug(struct work_struct *work)
>> {
>> - struct exynos_dp_device *dp;
>> + struct analogix_dp_device *dp;
>>
>> - dp = container_of(work, struct exynos_dp_device, hotplug_work);
>> + dp = container_of(work, struct analogix_dp_device, hotplug_work);
>>
>> if (dp->drm_dev)
>> drm_helper_hpd_irq_event(dp->drm_dev);
>> }
>>
>> -static void exynos_dp_commit(struct drm_encoder *encoder)
>> +static void analogix_dp_commit(struct analogix_dp_device *dp)
>> {
>> - struct exynos_dp_device *dp = encoder_to_dp(encoder);
>> int ret;
>>
>> /* Keep the panel disabled while we configure video */
>> - if (dp->panel) {
>> - if (drm_panel_disable(dp->panel))
>> + if (dp->plat_data && dp->plat_data->panel) {
>> + if (drm_panel_disable(dp->plat_data->panel))
>> DRM_ERROR("failed to disable the panel\n");
>> }
>>
>> - ret = exynos_dp_detect_hpd(dp);
>> + ret = analogix_dp_detect_hpd(dp);
>> if (ret) {
>> /* Cable has been disconnected, we're done */
>> return;
>> }
>>
>> - ret = exynos_dp_handle_edid(dp);
>> + ret = analogix_dp_handle_edid(dp);
>> if (ret) {
>> dev_err(dp->dev, "unable to handle edid\n");
>> return;
>> }
>>
>> - ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
>> - dp->video_info->link_rate);
>> + ret = analogix_dp_set_link_train(dp, dp->video_info->lane_count,
>> + dp->video_info->link_rate);
>> if (ret) {
>> dev_err(dp->dev, "unable to do link train\n");
>> return;
>> }
>>
>> - exynos_dp_enable_scramble(dp, 1);
>> - exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
>> - exynos_dp_enable_enhanced_mode(dp, 1);
>> + analogix_dp_enable_scramble(dp, 1);
>> + analogix_dp_enable_rx_to_enhanced_mode(dp, 1);
>> + analogix_dp_enable_enhanced_mode(dp, 1);
>>
>> - exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
>> - exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
>> + analogix_dp_set_lane_count(dp, dp->video_info->lane_count);
>> + analogix_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
>>
>> - exynos_dp_init_video(dp);
>> - ret = exynos_dp_config_video(dp);
>> + analogix_dp_init_video(dp);
>> + ret = analogix_dp_config_video(dp);
>> if (ret)
>> dev_err(dp->dev, "unable to config video\n");
>>
>> /* Safe to enable the panel now */
>> - if (dp->panel) {
>> - if (drm_panel_enable(dp->panel))
>> + if (dp->plat_data && dp->plat_data->panel) {
>> + if (drm_panel_enable(dp->plat_data->panel))
>> DRM_ERROR("failed to enable the panel\n");
>> }
>>
>> /* Enable video */
>> - exynos_dp_start_video(dp);
>> + analogix_dp_start_video(dp);
>> }
>>
>> static enum drm_connector_status
>> -exynos_dp_detect(struct drm_connector *connector, bool force)
>> +analogix_dp_detect(struct drm_connector *connector, bool force)
>> {
>> return connector_status_connected;
>> }
>>
>> -static void exynos_dp_connector_destroy(struct drm_connector *connector)
>> +static void analogix_dp_connector_destroy(struct drm_connector *connector)
>> {
>> drm_connector_unregister(connector);
>> drm_connector_cleanup(connector);
>> }
>>
>> -static struct drm_connector_funcs exynos_dp_connector_funcs = {
>> +static struct drm_connector_funcs analogix_dp_connector_funcs = {
>> .dpms = drm_atomic_helper_connector_dpms,
>> .fill_modes = drm_helper_probe_single_connector_modes,
>> - .detect = exynos_dp_detect,
>> - .destroy = exynos_dp_connector_destroy,
>> + .detect = analogix_dp_detect,
>> + .destroy = analogix_dp_connector_destroy,
>> .reset = drm_atomic_helper_connector_reset,
>> .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> };
>>
>> -static int exynos_dp_get_modes(struct drm_connector *connector)
>> +static int analogix_dp_get_modes(struct drm_connector *connector)
>> {
>> - struct exynos_dp_device *dp = ctx_from_connector(connector);
>> - struct drm_display_mode *mode;
>> -
>> - if (dp->panel)
>> - return drm_panel_get_modes(dp->panel);
>> + struct analogix_dp_device *dp = connector_to_dp(connector);
>> + struct analogix_dp_plat_data *plat_data = dp->plat_data;
>> + int num_modes = 0;
>>
>> - mode = drm_mode_create(connector->dev);
>> - if (!mode) {
>> - DRM_ERROR("failed to create a new display mode.\n");
>> - return 0;
>> - }
>> -
>> - drm_display_mode_from_videomode(&dp->priv.vm, mode);
>> - mode->width_mm = dp->priv.width_mm;
>> - mode->height_mm = dp->priv.height_mm;
>> - connector->display_info.width_mm = mode->width_mm;
>> - connector->display_info.height_mm = mode->height_mm;
>> + if (plat_data && plat_data->panel)
>> + num_modes += drm_panel_get_modes(plat_data->panel);
>>
>> - mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
>> - drm_mode_set_name(mode);
>> - drm_mode_probed_add(connector, mode);
>> + if (plat_data && plat_data->get_modes)
>> + num_modes += plat_data->get_modes(plat_data, connector);
>>
>> - return 1;
>> + return num_modes;
>> }
>>
>> static struct drm_encoder *
>> -exynos_dp_best_encoder(struct drm_connector *connector)
>> +analogix_dp_best_encoder(struct drm_connector *connector)
>> {
>> - struct exynos_dp_device *dp = ctx_from_connector(connector);
>> + struct analogix_dp_device *dp = connector_to_dp(connector);
>>
>> - return &dp->encoder;
>> + return dp->encoder;
>> }
>>
>> -static struct drm_connector_helper_funcs exynos_dp_connector_helper_funcs = {
>> - .get_modes = exynos_dp_get_modes,
>> - .best_encoder = exynos_dp_best_encoder,
>> +static struct drm_connector_helper_funcs analogix_dp_connector_helper_funcs = {
>> + .get_modes = analogix_dp_get_modes,
>> + .best_encoder = analogix_dp_best_encoder,
>> };
>>
>> -/* returns the number of bridges attached */
>> -static int exynos_drm_attach_lcd_bridge(struct exynos_dp_device *dp,
>> - struct drm_encoder *encoder)
>> -{
>> - int ret;
>> -
>> - encoder->bridge->next = dp->ptn_bridge;
>> - dp->ptn_bridge->encoder = encoder;
>> - ret = drm_bridge_attach(encoder->dev, dp->ptn_bridge);
>> - if (ret) {
>> - DRM_ERROR("Failed to attach bridge to drm\n");
>> - return ret;
>> - }
>> -
>> - return 0;
>> -}
>> -
>> -static int exynos_dp_bridge_attach(struct drm_bridge *bridge)
>> +static int analogix_dp_bridge_attach(struct drm_bridge *bridge)
>> {
>> - struct exynos_dp_device *dp = bridge->driver_private;
>> - struct drm_encoder *encoder = &dp->encoder;
>> + struct analogix_dp_device *dp = bridge->driver_private;
>> + struct drm_encoder *encoder = dp->encoder;
>> struct drm_connector *connector = &dp->connector;
>> int ret;
>>
>> - /* Pre-empt DP connector creation if there's a bridge */
>> - if (dp->ptn_bridge) {
>> - ret = exynos_drm_attach_lcd_bridge(dp, encoder);
>> - if (!ret)
>> - return 0;
>> + if (!bridge->encoder) {
>> + DRM_ERROR("Parent encoder object not found");
>> + return -ENODEV;
>> }
>>
>> + encoder->bridge = bridge;
>> +
>> connector->polled = DRM_CONNECTOR_POLL_HPD;
>>
>> ret = drm_connector_init(dp->drm_dev, connector,
>> - &exynos_dp_connector_funcs,
>> + &analogix_dp_connector_funcs,
>> DRM_MODE_CONNECTOR_eDP);
>> if (ret) {
>> DRM_ERROR("Failed to initialize connector with drm\n");
>> return ret;
>> }
>>
>> - drm_connector_helper_add(connector, &exynos_dp_connector_helper_funcs);
>> + drm_connector_helper_add(connector,
>> + &analogix_dp_connector_helper_funcs);
>> drm_connector_register(connector);
>> drm_mode_connector_attach_encoder(connector, encoder);
>>
>> - if (dp->panel)
>> - ret = drm_panel_attach(dp->panel, &dp->connector);
>> + if (dp->plat_data && dp->plat_data->panel) {
>> + ret = drm_panel_attach(dp->plat_data->panel, &dp->connector);
>> + if (ret) {
>> + DRM_ERROR("Failed to attach panel\n");
>> + return ret;
>> + }
>> + }
>> +
>> + /*
>> + * This should be the end of attach function, caused
>> + * we should ensure dp bridge could attach first.
>> + */
>> + if (dp->plat_data && dp->plat_data->attach) {
>> + ret = dp->plat_data->attach(dp->plat_data, bridge);
>> + if (ret) {
>> + DRM_ERROR("Failed at platform attch func\n");
> Two new error paths appeared here and above. Don't you have to
> cleanup something? I don't know, just wondering...
Hmm... I think both panel & platform_attch need ERROR remind when
it failed. But if it still need clean, I though it is tge platform attch
error,
this is not relate to DRM directly, just analogix driver logic, so code
would
like.
- if (dp->panel)
- ret = drm_panel_attach(dp->panel, &dp->connector);
+ if (dp->plat_data && dp->plat_data->panel) {
+ ret = drm_panel_attach(dp->plat_data->panel, &dp->connector);
+ if (ret) {
+ DRM_ERROR("Failed to attach panel\n");
+ return ret;
+ }
+ }
+ /*
+ * This should be the end of attach function, caused
+ * we should ensure dp bridge could attach first.
+ */
+ if (dp->plat_data && dp->plat_data->attach) {
+ ret = dp->plat_data->attach(dp->plat_data, bridge);
return ret;
> (...)
>
>> @@ -1345,22 +1286,9 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
>> disable_irq(dp->irq);
>>
>> dp->drm_dev = drm_dev;
>> + dp->encoder = encoder;
>>
>> - pipe = exynos_drm_crtc_get_pipe_from_type(drm_dev,
>> - EXYNOS_DISPLAY_TYPE_LCD);
>> - if (pipe < 0)
>> - return pipe;
>> -
>> - encoder->possible_crtcs = 1 << pipe;
>> -
>> - DRM_DEBUG_KMS("possible_crtcs = 0x%x\n", encoder->possible_crtcs);
>> -
>> - drm_encoder_init(drm_dev, encoder, &exynos_dp_encoder_funcs,
>> - DRM_MODE_ENCODER_TMDS);
>> -
>> - drm_encoder_helper_add(encoder, &exynos_dp_encoder_helper_funcs);
>> -
>> - ret = exynos_dp_create_connector(encoder);
>> + ret = analogix_dp_create_bridge(drm_dev, dp);
>> if (ret) {
>> DRM_ERROR("failed to create connector ret = %d\n", ret);
>> drm_encoder_cleanup(encoder);
>> @@ -1369,103 +1297,36 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
>>
>> return 0;
>> }
>> +EXPORT_SYMBOL_GPL(analogix_dp_bind);
> Ooo, a new export symbol. I suppose it is necessary for the split
> but the commit message does not mention exporting
> anything nor the reason for doing it. Could you extend the commit
> message describing more tha changes here and rationale for it?
Okay, got it.
> (...)
>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> new file mode 100644
>> index 0000000..b879d8c
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> @@ -0,0 +1,1265 @@
>> +/*
>> + * Analogix DP (Display port) core register interface driver.
>> + *
>> + * Copyright (C) 2012 Samsung Electronics Co., Ltd.
>> + * Author: Jingoo Han <jg1.han@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License as published by the
>> + * Free Software Foundation; either version 2 of the License, or (at your
>> + * option) any later version.
>> + */
>> +
>> +#include <linux/device.h>
>> +#include <linux/io.h>
>> +#include <linux/delay.h>
>> +#include <linux/gpio.h>
>> +
>> +#include "analogix_dp_core.h"
>> +#include "analogix_dp_reg.h"
>> +
>> +#define COMMON_INT_MASK_1 0
>> +#define COMMON_INT_MASK_2 0
>> +#define COMMON_INT_MASK_3 0
>> +#define COMMON_INT_MASK_4 (HOTPLUG_CHG | HPD_LOST | PLUG)
>> +#define INT_STA_MASK INT_HPD
>> +
>> +void analogix_dp_enable_video_mute(struct analogix_dp_device *dp, bool enable)
>> +{
>> + u32 reg;
>> +
>> + if (enable) {
>> + reg = readl(dp->reg_base + ANALOGIX_DP_VIDEO_CTL_1);
>> + reg |= HDCP_VIDEO_MUTE;
>> + writel(reg, dp->reg_base + ANALOGIX_DP_VIDEO_CTL_1);
>> + } else {
>> + reg = readl(dp->reg_base + ANALOGIX_DP_VIDEO_CTL_1);
>> + reg &= ~HDCP_VIDEO_MUTE;
>> + writel(reg, dp->reg_base + ANALOGIX_DP_VIDEO_CTL_1);
>> + }
>> +}
>> +
>> +void analogix_dp_stop_video(struct analogix_dp_device *dp)
>> +{
>> + u32 reg;
>> +
>> + reg = readl(dp->reg_base + ANALOGIX_DP_VIDEO_CTL_1);
>> + reg &= ~VIDEO_EN;
>> + writel(reg, dp->reg_base + ANALOGIX_DP_VIDEO_CTL_1);
>> +}
>> +
>> +void analogix_dp_lane_swap(struct analogix_dp_device *dp, bool enable)
>> +{
>> + u32 reg;
>> +
>> + if (enable)
>> + reg = LANE3_MAP_LOGIC_LANE_0 | LANE2_MAP_LOGIC_LANE_1 |
>> + LANE1_MAP_LOGIC_LANE_2 | LANE0_MAP_LOGIC_LANE_3;
>> + else
>> + reg = LANE3_MAP_LOGIC_LANE_3 | LANE2_MAP_LOGIC_LANE_2 |
>> + LANE1_MAP_LOGIC_LANE_1 | LANE0_MAP_LOGIC_LANE_0;
> Here and in few other places with new code: no alignment?
>
> You spent some time fixing these in patch 1, so I would assume
> that new code already comes after "checkpatch -f --subjective".
> That's actually the only valid reason for using checkpatch - new code.
> However my mail reader could corrupt the output so maybe this is aligned?
This is another mistaken I make, and surprise by that checkpatch
fine with this no-alignment style. Anyway I would just do rename
change in this split patch, and move the style change to another
separate patch (maybe I should adjust the order, correct style
problem after this patch).
> Code seems fine after looking at it but review was rather shallow.
> I don't know the Exynos DP driver.
I really appreciate for your careful review ;)
- Yakir
> Best regards,
> Krzysztof
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Krzysztof Kozlowski <k.kozlowski@samsung.com> |
|---|---|
| Date | 2015-09-04 02:50 +0200 |
| Subject | Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir |
| Message-ID | <q4Nmy-31s-7@gated-at.bofh.it> |
| In reply to | #1218046 |
On 03.09.2015 14:30, Yakir Yang wrote:
> Hi Krzysztof,
>
> 在 09/03/2015 08:58 AM, Krzysztof Kozlowski 写道:
>> On 01.09.2015 14:49, Yakir Yang wrote:
>>> Split the dp core driver from exynos directory to bridge
>>> directory, and rename the core driver to analogix_dp_*,
>>> leave the platform code to analogix_dp-exynos.
>>>
>>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>>> ---
>>> Changes in v4:
>>> - Take Rob suggest, update "analogix,hpd-gpios" to "hpd-gpios" DT
>>> propery.
>>> - Take Jingoo suggest, rename "analogix_dp-exynos.c" file name to
>>> "exynos_dp.c"
>>> - Take Archit suggest, create a separate folder for analogix code in
>>> bridge/
>>>
>>> Changes in v3:
>>> - Take Thierry Reding suggest, move exynos's video_timing code
>>> to analogix_dp-exynos platform driver, add get_modes method
>>> to struct analogix_dp_plat_data.
>>> - Take Heiko suggest, rename some "samsung*" dts propery to "analogix*".
>>>
>>> Changes in v2:
>>> - Take Jingoo Han suggest, remove new copyright
>>> - Fix compiled failed dut to analogix_dp_device misspell
>>>
>>> drivers/gpu/drm/bridge/Kconfig | 2 +
>>> drivers/gpu/drm/bridge/Makefile | 1 +
>>> drivers/gpu/drm/bridge/analogix/Kconfig | 4 +
>>> drivers/gpu/drm/bridge/analogix/Makefile | 1 +
>>> .../analogix/analogix_dp_core.c} | 817
>>> ++++++-------
>>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 283 +++++
>>> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 1265
>>> ++++++++++++++++++++
>>> .../analogix/analogix_dp_reg.h} | 258 ++--
>>> drivers/gpu/drm/exynos/Kconfig | 3 +-
>>> drivers/gpu/drm/exynos/Makefile | 2 +-
>>> drivers/gpu/drm/exynos/exynos_dp.c | 306 +++++
>>> drivers/gpu/drm/exynos/exynos_dp_core.h | 282 -----
>>> drivers/gpu/drm/exynos/exynos_dp_reg.c | 1259
>>> -------------------
>>> include/drm/bridge/analogix_dp.h | 24 +
>>> 14 files changed, 2357 insertions(+), 2150 deletions(-)
>>> create mode 100644 drivers/gpu/drm/bridge/analogix/Kconfig
>>> create mode 100644 drivers/gpu/drm/bridge/analogix/Makefile
>>> rename drivers/gpu/drm/{exynos/exynos_dp_core.c =>
>>> bridge/analogix/analogix_dp_core.c} (50%)
>>> create mode 100644 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>>> create mode 100644 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>>> rename drivers/gpu/drm/{exynos/exynos_dp_reg.h =>
>>> bridge/analogix/analogix_dp_reg.h} (64%)
>>> create mode 100644 drivers/gpu/drm/exynos/exynos_dp.c
>>> delete mode 100644 drivers/gpu/drm/exynos/exynos_dp_core.h
>>> delete mode 100644 drivers/gpu/drm/exynos/exynos_dp_reg.c
>>> create mode 100644 include/drm/bridge/analogix_dp.h
>>>
>>> diff --git a/drivers/gpu/drm/bridge/Kconfig
>>> b/drivers/gpu/drm/bridge/Kconfig
>>> index 2de52a5..7b5b77a 100644
>>> --- a/drivers/gpu/drm/bridge/Kconfig
>>> +++ b/drivers/gpu/drm/bridge/Kconfig
>>> @@ -29,4 +29,6 @@ config DRM_PARADE_PS8622
>>> ---help---
>>> Parade eDP-LVDS bridge chip driver.
>>> +source "drivers/gpu/drm/bridge/analogix/Kconfig"
>>> +
>>> endmenu
>>> diff --git a/drivers/gpu/drm/bridge/Makefile
>>> b/drivers/gpu/drm/bridge/Makefile
>>> index e2eef1c..5366c6b 100644
>>> --- a/drivers/gpu/drm/bridge/Makefile
>>> +++ b/drivers/gpu/drm/bridge/Makefile
>>> @@ -3,3 +3,4 @@ ccflags-y := -Iinclude/drm
>>> obj-$(CONFIG_DRM_DW_HDMI) += dw_hdmi.o
>>> obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
>>> obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
>>> +obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>>> diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig
>>> b/drivers/gpu/drm/bridge/analogix/Kconfig
>>> new file mode 100644
>>> index 0000000..5ff6551
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/bridge/analogix/Kconfig
>>> @@ -0,0 +1,4 @@
>>> +config DRM_ANALOGIX_DP
>>> + tristate
>>> + depends on DRM
>>> + select DRM_KMS_HELPER
>>> diff --git a/drivers/gpu/drm/bridge/analogix/Makefile
>>> b/drivers/gpu/drm/bridge/analogix/Makefile
>>> new file mode 100644
>>> index 0000000..9107b86
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/bridge/analogix/Makefile
>>> @@ -0,0 +1 @@
>>> +obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp_core.o analogix_dp_reg.o
>>> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c
>>> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> similarity index 50%
>>> rename from drivers/gpu/drm/exynos/exynos_dp_core.c
>>> rename to drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> index bed0252..7d62f22 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> @@ -1,14 +1,14 @@
>>> /*
>>> - * Samsung SoC DP (Display Port) interface driver.
>>> - *
>>> - * Copyright (C) 2012 Samsung Electronics Co., Ltd.
>>> - * Author: Jingoo Han <jg1.han@samsung.com>
>>> - *
>>> - * This program is free software; you can redistribute it and/or
>>> modify it
>>> - * under the terms of the GNU General Public License as published by
>>> the
>>> - * Free Software Foundation; either version 2 of the License, or (at
>>> your
>>> - * option) any later version.
>>> - */
>>> +* Analogix DP (Display Port) core interface driver.
>>> +*
>>> +* Copyright (C) 2012 Samsung Electronics Co., Ltd.
>>> +* Author: Jingoo Han <jg1.han@samsung.com>
>>> +*
>>> +* This program is free software; you can redistribute it and/or
>>> modify it
>>> +* under the terms of the GNU General Public License as published by the
>>> +* Free Software Foundation; either version 2 of the License, or (at
>>> your
>>> +* option) any later version.
>>> +*/
>>> #include <linux/module.h>
>>> #include <linux/platform_device.h>
>>> @@ -18,12 +18,9 @@
>>> #include <linux/interrupt.h>
>>> #include <linux/of.h>
>>> #include <linux/of_gpio.h>
>>> -#include <linux/of_graph.h>
>>> #include <linux/gpio.h>
>>> #include <linux/component.h>
>>> #include <linux/phy/phy.h>
>>> -#include <video/of_display_timing.h>
>>> -#include <video/of_videomode.h>
>>> #include <drm/drmP.h>
>>> #include <drm/drm_crtc.h>
>>> @@ -31,52 +28,42 @@
>>> #include <drm/drm_atomic_helper.h>
>>> #include <drm/drm_panel.h>
>>> -#include "exynos_dp_core.h"
>>> -#include "exynos_drm_crtc.h"
>>> +#include <drm/bridge/analogix_dp.h>
>>> -#define ctx_from_connector(c) container_of(c, struct
>>> exynos_dp_device, \
>>> - connector)
>>> +#include "analogix_dp_core.h"
>>> -static inline struct exynos_drm_crtc *dp_to_crtc(struct
>>> exynos_dp_device *dp)
>>> -{
>>> - return to_exynos_crtc(dp->encoder.crtc);
>>> -}
>>> -
>>> -static inline struct exynos_dp_device *encoder_to_dp(
>>> - struct drm_encoder *e)
>>> -{
>>> - return container_of(e, struct exynos_dp_device, encoder);
>>> -}
>>> +#define connector_to_dp(c) \
>>> + container_of(c, struct analogix_dp_device, connector)
>>> struct bridge_init {
>>> struct i2c_client *client;
>>> struct device_node *node;
>>> };
>>> -static void exynos_dp_init_dp(struct exynos_dp_device *dp)
>>> +static void analogix_dp_init_dp(struct analogix_dp_device *dp)
>>> {
>>> - exynos_dp_reset(dp);
>>> + analogix_dp_reset(dp);
>>> - exynos_dp_swreset(dp);
>>> + analogix_dp_swreset(dp);
>>> - exynos_dp_init_analog_param(dp);
>>> - exynos_dp_init_interrupt(dp);
>>> + analogix_dp_init_analog_param(dp);
>>> + analogix_dp_init_interrupt(dp);
>>> /* SW defined function Normal operation */
>>> - exynos_dp_enable_sw_function(dp);
>>> + analogix_dp_enable_sw_function(dp);
>>> - exynos_dp_config_interrupt(dp);
>>> - exynos_dp_init_analog_func(dp);
>>> + analogix_dp_config_interrupt(dp);
>>> + analogix_dp_init_analog_func(dp);
>>> - exynos_dp_init_hpd(dp);
>>> - exynos_dp_init_aux(dp);
>>> + analogix_dp_init_hpd(dp);
>>> + analogix_dp_init_aux(dp);
>>> }
>>> -static int exynos_dp_detect_hpd(struct exynos_dp_device *dp)
>>> +static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
>>> {
>>> int timeout_loop = 0;
>>> - while (exynos_dp_get_plug_in_status(dp) != 0) {
>>> + while (analogix_dp_get_plug_in_status(dp) != 0) {
>>> timeout_loop++;
>>> if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
>>> dev_err(dp->dev, "failed to get hpd plug status\n");
>>> @@ -88,7 +75,7 @@ static int exynos_dp_detect_hpd(struct
>>> exynos_dp_device *dp)
>>> return 0;
>>> }
>>> -static unsigned char exynos_dp_calc_edid_check_sum(unsigned char
>>> *edid_data)
>>> +static unsigned char analogix_dp_calc_edid_check_sum(unsigned char
>>> *edid_data)
>>> {
>>> int i;
>>> unsigned char sum = 0;
>>> @@ -99,7 +86,7 @@ static unsigned char
>>> exynos_dp_calc_edid_check_sum(unsigned char *edid_data)
>>> return sum;
>>> }
>>> -static int exynos_dp_read_edid(struct exynos_dp_device *dp)
>>> +static int analogix_dp_read_edid(struct analogix_dp_device *dp)
>>> {
>>> unsigned char edid[EDID_BLOCK_LENGTH * 2];
>>> unsigned int extend_block = 0;
>>> @@ -114,9 +101,9 @@ static int exynos_dp_read_edid(struct
>>> exynos_dp_device *dp)
>>> */
>>> /* Read Extension Flag, Number of 128-byte EDID extension
>>> blocks */
>>> - retval = exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
>>> - EDID_EXTENSION_FLAG,
>>> - &extend_block);
>>> + retval = analogix_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
>>> + EDID_EXTENSION_FLAG,
>>> + &extend_block);
>> Hi,
>>
>> What was the point of patch 1 (checkpatch -f --subjective) if you
>> have to re-align the parameters again? It's meaningless.
>>
>> I could understand that idea if this patch touched only first
>> line - name of the function. This would result in small
>> and effective diff, like:
>>
>> - retval = exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
>> + retval = SAME_LEGNTH____byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
>> EDID_EXTENSION_FLAG,
>>
>> But this is not the case. The reviewer have to look at 6 lines
>> of changes instead of two. You fixed the alignment just for two
>> patches and then you re-written it. It's meaningless.
>
> Yes, this is the place that crazy me too. After rename "exynos"
> to "analogix", there would bring some "over-80-char" problem :(
If you can't avoid it then it's okay. When you rename the function this
could happen and then you just re-align the block.
>
>> Nope. Please drop all changes from patch 1 which:
>> (all conditions apply):
>> 1. change alignment,
>> 2. are immediately changed in next patches (e.g. like here),
>>
>> and fix the alignment while renaming the function.
>>
>> This is actually another point for not accepting commits where
>> the reason is "checkpatch told me".
>
> Okay, so there would be two patches to fix the alignment, done.
>
> Thanks for your careful remind.
Two patches but fixing alignment in different places. Do not fix the
alignment in 1/16 and then re-align the same line at 3/16.
>
>>
>>
>>> if (retval)
>>> return retval;
>>> @@ -124,7 +111,7 @@ static int exynos_dp_read_edid(struct
>>> exynos_dp_device *dp)
>>> dev_dbg(dp->dev, "EDID data includes a single extension!\n");
>>> /* Read EDID data */
>>> - retval = exynos_dp_read_bytes_from_i2c(
>>> + retval = analogix_dp_read_bytes_from_i2c(
>>> dp, I2C_EDID_DEVICE_ADDR,
>>> EDID_HEADER_PATTERN,
>>> EDID_BLOCK_LENGTH,
>>> @@ -133,14 +120,14 @@ static int exynos_dp_read_edid(struct
>>> exynos_dp_device *dp)
>> (...)
>>
>>> @@ -871,218 +862,204 @@ static irqreturn_t exynos_dp_irq_handler(int
>>> irq, void *arg)
>>> return IRQ_HANDLED;
>>> }
>>> -static void exynos_dp_hotplug(struct work_struct *work)
>>> +static void analogix_dp_hotplug(struct work_struct *work)
>>> {
>>> - struct exynos_dp_device *dp;
>>> + struct analogix_dp_device *dp;
>>> - dp = container_of(work, struct exynos_dp_device, hotplug_work);
>>> + dp = container_of(work, struct analogix_dp_device, hotplug_work);
>>> if (dp->drm_dev)
>>> drm_helper_hpd_irq_event(dp->drm_dev);
>>> }
>>> -static void exynos_dp_commit(struct drm_encoder *encoder)
>>> +static void analogix_dp_commit(struct analogix_dp_device *dp)
>>> {
>>> - struct exynos_dp_device *dp = encoder_to_dp(encoder);
>>> int ret;
>>> /* Keep the panel disabled while we configure video */
>>> - if (dp->panel) {
>>> - if (drm_panel_disable(dp->panel))
>>> + if (dp->plat_data && dp->plat_data->panel) {
>>> + if (drm_panel_disable(dp->plat_data->panel))
>>> DRM_ERROR("failed to disable the panel\n");
>>> }
>>> - ret = exynos_dp_detect_hpd(dp);
>>> + ret = analogix_dp_detect_hpd(dp);
>>> if (ret) {
>>> /* Cable has been disconnected, we're done */
>>> return;
>>> }
>>> - ret = exynos_dp_handle_edid(dp);
>>> + ret = analogix_dp_handle_edid(dp);
>>> if (ret) {
>>> dev_err(dp->dev, "unable to handle edid\n");
>>> return;
>>> }
>>> - ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
>>> - dp->video_info->link_rate);
>>> + ret = analogix_dp_set_link_train(dp, dp->video_info->lane_count,
>>> + dp->video_info->link_rate);
>>> if (ret) {
>>> dev_err(dp->dev, "unable to do link train\n");
>>> return;
>>> }
>>> - exynos_dp_enable_scramble(dp, 1);
>>> - exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
>>> - exynos_dp_enable_enhanced_mode(dp, 1);
>>> + analogix_dp_enable_scramble(dp, 1);
>>> + analogix_dp_enable_rx_to_enhanced_mode(dp, 1);
>>> + analogix_dp_enable_enhanced_mode(dp, 1);
>>> - exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
>>> - exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
>>> + analogix_dp_set_lane_count(dp, dp->video_info->lane_count);
>>> + analogix_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
>>> - exynos_dp_init_video(dp);
>>> - ret = exynos_dp_config_video(dp);
>>> + analogix_dp_init_video(dp);
>>> + ret = analogix_dp_config_video(dp);
>>> if (ret)
>>> dev_err(dp->dev, "unable to config video\n");
>>> /* Safe to enable the panel now */
>>> - if (dp->panel) {
>>> - if (drm_panel_enable(dp->panel))
>>> + if (dp->plat_data && dp->plat_data->panel) {
>>> + if (drm_panel_enable(dp->plat_data->panel))
>>> DRM_ERROR("failed to enable the panel\n");
>>> }
>>> /* Enable video */
>>> - exynos_dp_start_video(dp);
>>> + analogix_dp_start_video(dp);
>>> }
>>> static enum drm_connector_status
>>> -exynos_dp_detect(struct drm_connector *connector, bool force)
>>> +analogix_dp_detect(struct drm_connector *connector, bool force)
>>> {
>>> return connector_status_connected;
>>> }
>>> -static void exynos_dp_connector_destroy(struct drm_connector
>>> *connector)
>>> +static void analogix_dp_connector_destroy(struct drm_connector
>>> *connector)
>>> {
>>> drm_connector_unregister(connector);
>>> drm_connector_cleanup(connector);
>>> }
>>> -static struct drm_connector_funcs exynos_dp_connector_funcs = {
>>> +static struct drm_connector_funcs analogix_dp_connector_funcs = {
>>> .dpms = drm_atomic_helper_connector_dpms,
>>> .fill_modes = drm_helper_probe_single_connector_modes,
>>> - .detect = exynos_dp_detect,
>>> - .destroy = exynos_dp_connector_destroy,
>>> + .detect = analogix_dp_detect,
>>> + .destroy = analogix_dp_connector_destroy,
>>> .reset = drm_atomic_helper_connector_reset,
>>> .atomic_duplicate_state =
>>> drm_atomic_helper_connector_duplicate_state,
>>> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>>> };
>>> -static int exynos_dp_get_modes(struct drm_connector *connector)
>>> +static int analogix_dp_get_modes(struct drm_connector *connector)
>>> {
>>> - struct exynos_dp_device *dp = ctx_from_connector(connector);
>>> - struct drm_display_mode *mode;
>>> -
>>> - if (dp->panel)
>>> - return drm_panel_get_modes(dp->panel);
>>> + struct analogix_dp_device *dp = connector_to_dp(connector);
>>> + struct analogix_dp_plat_data *plat_data = dp->plat_data;
>>> + int num_modes = 0;
>>> - mode = drm_mode_create(connector->dev);
>>> - if (!mode) {
>>> - DRM_ERROR("failed to create a new display mode.\n");
>>> - return 0;
>>> - }
>>> -
>>> - drm_display_mode_from_videomode(&dp->priv.vm, mode);
>>> - mode->width_mm = dp->priv.width_mm;
>>> - mode->height_mm = dp->priv.height_mm;
>>> - connector->display_info.width_mm = mode->width_mm;
>>> - connector->display_info.height_mm = mode->height_mm;
>>> + if (plat_data && plat_data->panel)
>>> + num_modes += drm_panel_get_modes(plat_data->panel);
>>> - mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
>>> - drm_mode_set_name(mode);
>>> - drm_mode_probed_add(connector, mode);
>>> + if (plat_data && plat_data->get_modes)
>>> + num_modes += plat_data->get_modes(plat_data, connector);
>>> - return 1;
>>> + return num_modes;
>>> }
>>> static struct drm_encoder *
>>> -exynos_dp_best_encoder(struct drm_connector *connector)
>>> +analogix_dp_best_encoder(struct drm_connector *connector)
>>> {
>>> - struct exynos_dp_device *dp = ctx_from_connector(connector);
>>> + struct analogix_dp_device *dp = connector_to_dp(connector);
>>> - return &dp->encoder;
>>> + return dp->encoder;
>>> }
>>> -static struct drm_connector_helper_funcs
>>> exynos_dp_connector_helper_funcs = {
>>> - .get_modes = exynos_dp_get_modes,
>>> - .best_encoder = exynos_dp_best_encoder,
>>> +static struct drm_connector_helper_funcs
>>> analogix_dp_connector_helper_funcs = {
>>> + .get_modes = analogix_dp_get_modes,
>>> + .best_encoder = analogix_dp_best_encoder,
>>> };
>>> -/* returns the number of bridges attached */
>>> -static int exynos_drm_attach_lcd_bridge(struct exynos_dp_device *dp,
>>> - struct drm_encoder *encoder)
>>> -{
>>> - int ret;
>>> -
>>> - encoder->bridge->next = dp->ptn_bridge;
>>> - dp->ptn_bridge->encoder = encoder;
>>> - ret = drm_bridge_attach(encoder->dev, dp->ptn_bridge);
>>> - if (ret) {
>>> - DRM_ERROR("Failed to attach bridge to drm\n");
>>> - return ret;
>>> - }
>>> -
>>> - return 0;
>>> -}
>>> -
>>> -static int exynos_dp_bridge_attach(struct drm_bridge *bridge)
>>> +static int analogix_dp_bridge_attach(struct drm_bridge *bridge)
>>> {
>>> - struct exynos_dp_device *dp = bridge->driver_private;
>>> - struct drm_encoder *encoder = &dp->encoder;
>>> + struct analogix_dp_device *dp = bridge->driver_private;
>>> + struct drm_encoder *encoder = dp->encoder;
>>> struct drm_connector *connector = &dp->connector;
>>> int ret;
>>> - /* Pre-empt DP connector creation if there's a bridge */
>>> - if (dp->ptn_bridge) {
>>> - ret = exynos_drm_attach_lcd_bridge(dp, encoder);
>>> - if (!ret)
>>> - return 0;
>>> + if (!bridge->encoder) {
>>> + DRM_ERROR("Parent encoder object not found");
>>> + return -ENODEV;
>>> }
>>> + encoder->bridge = bridge;
>>> +
>>> connector->polled = DRM_CONNECTOR_POLL_HPD;
>>> ret = drm_connector_init(dp->drm_dev, connector,
>>> - &exynos_dp_connector_funcs,
>>> + &analogix_dp_connector_funcs,
>>> DRM_MODE_CONNECTOR_eDP);
>>> if (ret) {
>>> DRM_ERROR("Failed to initialize connector with drm\n");
>>> return ret;
>>> }
>>> - drm_connector_helper_add(connector,
>>> &exynos_dp_connector_helper_funcs);
>>> + drm_connector_helper_add(connector,
>>> + &analogix_dp_connector_helper_funcs);
>>> drm_connector_register(connector);
>>> drm_mode_connector_attach_encoder(connector, encoder);
>>> - if (dp->panel)
>>> - ret = drm_panel_attach(dp->panel, &dp->connector);
>>> + if (dp->plat_data && dp->plat_data->panel) {
>>> + ret = drm_panel_attach(dp->plat_data->panel, &dp->connector);
>>> + if (ret) {
>>> + DRM_ERROR("Failed to attach panel\n");
>>> + return ret;
>>> + }
>>> + }
>>> +
>>> + /*
>>> + * This should be the end of attach function, caused
>>> + * we should ensure dp bridge could attach first.
>>> + */
>>> + if (dp->plat_data && dp->plat_data->attach) {
>>> + ret = dp->plat_data->attach(dp->plat_data, bridge);
>>> + if (ret) {
>>> + DRM_ERROR("Failed at platform attch func\n");
>> Two new error paths appeared here and above. Don't you have to
>> cleanup something? I don't know, just wondering...
>
> Hmm... I think both panel & platform_attch need ERROR remind when
> it failed. But if it still need clean, I though it is tge platform attch
> error,
> this is not relate to DRM directly, just analogix driver logic, so code
> would
> like.
>
> - if (dp->panel)
> - ret = drm_panel_attach(dp->panel, &dp->connector);
> + if (dp->plat_data && dp->plat_data->panel) {
> + ret = drm_panel_attach(dp->plat_data->panel, &dp->connector);
> + if (ret) {
> + DRM_ERROR("Failed to attach panel\n");
> + return ret;
> + }
> + }
>
> + /*
> + * This should be the end of attach function, caused
> + * we should ensure dp bridge could attach first.
> + */
> + if (dp->plat_data && dp->plat_data->attach) {
> + ret = dp->plat_data->attach(dp->plat_data, bridge);
>
> return ret;
I am lost... the code looks the same. What did you change?
Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2015-09-06 10:00 +0200 |
| Subject | Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir |
| Message-ID | <q5D1L-YF-7@gated-at.bofh.it> |
| In reply to | #1218626 |
Hi Krzysztof,
在 09/04/2015 08:41 AM, Krzysztof Kozlowski 写道:
> On 03.09.2015 14:30, Yakir Yang wrote:
>> Hi Krzysztof,
>>
>> 在 09/03/2015 08:58 AM, Krzysztof Kozlowski 写道:
>>> On 01.09.2015 14:49, Yakir Yang wrote:
>>>> Split the dp core driver from exynos directory to bridge
>>>> directory, and rename the core driver to analogix_dp_*,
>>>> leave the platform code to analogix_dp-exynos.
>>>>
>>>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>>>> ---
>>>> Changes in v4:
>>>> - Take Rob suggest, update "analogix,hpd-gpios" to "hpd-gpios" DT
>>>> propery.
>>>> - Take Jingoo suggest, rename "analogix_dp-exynos.c" file name to
>>>> "exynos_dp.c"
>>>> - Take Archit suggest, create a separate folder for analogix code in
>>>> bridge/
>>>>
>>>> Changes in v3:
>>>> - Take Thierry Reding suggest, move exynos's video_timing code
>>>> to analogix_dp-exynos platform driver, add get_modes method
>>>> to struct analogix_dp_plat_data.
>>>> - Take Heiko suggest, rename some "samsung*" dts propery to "analogix*".
>>>>
>>>> Changes in v2:
>>>> - Take Jingoo Han suggest, remove new copyright
>>>> - Fix compiled failed dut to analogix_dp_device misspell
[.....]
>>>> -static int exynos_dp_bridge_attach(struct drm_bridge *bridge)
>>>> +static int analogix_dp_bridge_attach(struct drm_bridge *bridge)
>>>> {
>>>> - struct exynos_dp_device *dp = bridge->driver_private;
>>>> - struct drm_encoder *encoder = &dp->encoder;
>>>> + struct analogix_dp_device *dp = bridge->driver_private;
>>>> + struct drm_encoder *encoder = dp->encoder;
>>>> struct drm_connector *connector = &dp->connector;
>>>> int ret;
>>>> - /* Pre-empt DP connector creation if there's a bridge */
>>>> - if (dp->ptn_bridge) {
>>>> - ret = exynos_drm_attach_lcd_bridge(dp, encoder);
>>>> - if (!ret)
>>>> - return 0;
>>>> + if (!bridge->encoder) {
>>>> + DRM_ERROR("Parent encoder object not found");
>>>> + return -ENODEV;
>>>> }
>>>> + encoder->bridge = bridge;
>>>> +
>>>> connector->polled = DRM_CONNECTOR_POLL_HPD;
>>>> ret = drm_connector_init(dp->drm_dev, connector,
>>>> - &exynos_dp_connector_funcs,
>>>> + &analogix_dp_connector_funcs,
>>>> DRM_MODE_CONNECTOR_eDP);
>>>> if (ret) {
>>>> DRM_ERROR("Failed to initialize connector with drm\n");
>>>> return ret;
>>>> }
>>>> - drm_connector_helper_add(connector,
>>>> &exynos_dp_connector_helper_funcs);
>>>> + drm_connector_helper_add(connector,
>>>> + &analogix_dp_connector_helper_funcs);
>>>> drm_connector_register(connector);
>>>> drm_mode_connector_attach_encoder(connector, encoder);
>>>> - if (dp->panel)
>>>> - ret = drm_panel_attach(dp->panel, &dp->connector);
>>>> + if (dp->plat_data && dp->plat_data->panel) {
>>>> + ret = drm_panel_attach(dp->plat_data->panel, &dp->connector);
>>>> + if (ret) {
>>>> + DRM_ERROR("Failed to attach panel\n");
>>>> + return ret;
>>>> + }
>>>> + }
>>>> +
>>>> + /*
>>>> + * This should be the end of attach function, caused
>>>> + * we should ensure dp bridge could attach first.
>>>> + */
>>>> + if (dp->plat_data && dp->plat_data->attach) {
>>>> + ret = dp->plat_data->attach(dp->plat_data, bridge);
>>>> + if (ret) {
>>>> + DRM_ERROR("Failed at platform attch func\n");
>>> Two new error paths appeared here and above. Don't you have to
>>> cleanup something? I don't know, just wondering...
>> Hmm... I think both panel & platform_attch need ERROR remind when
>> it failed. But if it still need clean, I though it should clean the platform attch
>> error,
>> this is not relate to DRM directly, just analogix driver logic, so code would like,
>>
>> - if (dp->panel)
>> - ret = drm_panel_attach(dp->panel, &dp->connector);
>> + if (dp->plat_data && dp->plat_data->panel) {
>> + ret = drm_panel_attach(dp->plat_data->panel, &dp->connector);
>> + if (ret) {
>> + DRM_ERROR("Failed to attach panel\n");
>> + return ret;
>> + }
>> + }
>>
>> + /*
>> + * This should be the end of attach function, caused
>> + * we should ensure dp bridge could attach first.
>> + */
>> + if (dp->plat_data && dp->plat_data->attach) {
>> + ret = dp->plat_data->attach(dp->plat_data, bridge);
>>
>> return ret;
> I am lost... the code looks the same. What did you change?
I just remove the DRM_ERROR after dp->plat_data->attach(),
maybe I should paste the change that rebase on this patch,
here are they,
/*
* This should be the end of attach function, caused
* we should ensure dp bridge could attach first.
*/
- if (dp->plat_data && dp->plat_data->attach) {
+ if (dp->plat_data && dp->plat_data->attach)
ret = dp->plat_data->attach(dp->plat_data, bridge);
- if (ret) {
- DRM_ERROR("Failed at platform attch func\n");
- return ret;
- }
- }
- return 0;
+ return ret;
If this haven't meet your comment, I maybe start to think that
your comment "Two new error paths appeared here and above"
indicated that those two function is the same.
"dp->plat_data->attach(dp->plat_data, bridge); "
"drm_panel_attach(dp->plat_data->panel, &dp->connector); "
Thanks,
- Yakir
> Best regards,
> Krzysztof
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Krzysztof Kozlowski <k.kozlowski@samsung.com> |
|---|---|
| Date | 2015-09-07 02:30 +0200 |
| Subject | Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir |
| Message-ID | <q5StQ-6dW-7@gated-at.bofh.it> |
| In reply to | #1219771 |
On 06.09.2015 16:49, Yakir Yang wrote:
> Hi Krzysztof,
>
> 在 09/04/2015 08:41 AM, Krzysztof Kozlowski 写道:
>> On 03.09.2015 14:30, Yakir Yang wrote:
>>> Hi Krzysztof,
>>>
>>> 在 09/03/2015 08:58 AM, Krzysztof Kozlowski 写道:
>>>> On 01.09.2015 14:49, Yakir Yang wrote:
>>>>> Split the dp core driver from exynos directory to bridge
>>>>> directory, and rename the core driver to analogix_dp_*,
>>>>> leave the platform code to analogix_dp-exynos.
>>>>>
>>>>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>>>>> ---
>>>>> Changes in v4:
>>>>> - Take Rob suggest, update "analogix,hpd-gpios" to "hpd-gpios" DT
>>>>> propery.
>>>>> - Take Jingoo suggest, rename "analogix_dp-exynos.c" file name to
>>>>> "exynos_dp.c"
>>>>> - Take Archit suggest, create a separate folder for analogix code in
>>>>> bridge/
>>>>>
>>>>> Changes in v3:
>>>>> - Take Thierry Reding suggest, move exynos's video_timing code
>>>>> to analogix_dp-exynos platform driver, add get_modes method
>>>>> to struct analogix_dp_plat_data.
>>>>> - Take Heiko suggest, rename some "samsung*" dts propery to
>>>>> "analogix*".
>>>>>
>>>>> Changes in v2:
>>>>> - Take Jingoo Han suggest, remove new copyright
>>>>> - Fix compiled failed dut to analogix_dp_device misspell
>
> [.....]
>
>>>>> -static int exynos_dp_bridge_attach(struct drm_bridge *bridge)
>>>>> +static int analogix_dp_bridge_attach(struct drm_bridge *bridge)
>>>>> {
>>>>> - struct exynos_dp_device *dp = bridge->driver_private;
>>>>> - struct drm_encoder *encoder = &dp->encoder;
>>>>> + struct analogix_dp_device *dp = bridge->driver_private;
>>>>> + struct drm_encoder *encoder = dp->encoder;
>>>>> struct drm_connector *connector = &dp->connector;
>>>>> int ret;
>>>>> - /* Pre-empt DP connector creation if there's a bridge */
>>>>> - if (dp->ptn_bridge) {
>>>>> - ret = exynos_drm_attach_lcd_bridge(dp, encoder);
>>>>> - if (!ret)
>>>>> - return 0;
>>>>> + if (!bridge->encoder) {
>>>>> + DRM_ERROR("Parent encoder object not found");
>>>>> + return -ENODEV;
>>>>> }
>>>>> + encoder->bridge = bridge;
>>>>> +
>>>>> connector->polled = DRM_CONNECTOR_POLL_HPD;
>>>>> ret = drm_connector_init(dp->drm_dev, connector,
>>>>> - &exynos_dp_connector_funcs,
>>>>> + &analogix_dp_connector_funcs,
>>>>> DRM_MODE_CONNECTOR_eDP);
>>>>> if (ret) {
>>>>> DRM_ERROR("Failed to initialize connector with drm\n");
>>>>> return ret;
>>>>> }
>>>>> - drm_connector_helper_add(connector,
>>>>> &exynos_dp_connector_helper_funcs);
>>>>> + drm_connector_helper_add(connector,
>>>>> + &analogix_dp_connector_helper_funcs);
>>>>> drm_connector_register(connector);
>>>>> drm_mode_connector_attach_encoder(connector, encoder);
>>>>> - if (dp->panel)
>>>>> - ret = drm_panel_attach(dp->panel, &dp->connector);
>>>>> + if (dp->plat_data && dp->plat_data->panel) {
>>>>> + ret = drm_panel_attach(dp->plat_data->panel, &dp->connector);
>>>>> + if (ret) {
>>>>> + DRM_ERROR("Failed to attach panel\n");
>>>>> + return ret;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + /*
>>>>> + * This should be the end of attach function, caused
>>>>> + * we should ensure dp bridge could attach first.
>>>>> + */
>>>>> + if (dp->plat_data && dp->plat_data->attach) {
>>>>> + ret = dp->plat_data->attach(dp->plat_data, bridge);
>>>>> + if (ret) {
>>>>> + DRM_ERROR("Failed at platform attch func\n");
>>>> Two new error paths appeared here and above. Don't you have to
>>>> cleanup something? I don't know, just wondering...
>>> Hmm... I think both panel & platform_attch need ERROR remind when
>>> it failed. But if it still need clean, I though it should clean the
>>> platform attch
>>> error,
>>> this is not relate to DRM directly, just analogix driver logic, so
>>> code would like,
>>>
>>> - if (dp->panel)
>>> - ret = drm_panel_attach(dp->panel, &dp->connector);
>>> + if (dp->plat_data && dp->plat_data->panel) {
>>> + ret = drm_panel_attach(dp->plat_data->panel, &dp->connector);
>>> + if (ret) {
>>> + DRM_ERROR("Failed to attach panel\n");
>>> + return ret;
>>> + }
>>> + }
>>>
>>> + /*
>>> + * This should be the end of attach function, caused
>>> + * we should ensure dp bridge could attach first.
>>> + */
>>> + if (dp->plat_data && dp->plat_data->attach) {
>>> + ret = dp->plat_data->attach(dp->plat_data, bridge);
>>>
>>> return ret;
>> I am lost... the code looks the same. What did you change?
>
> I just remove the DRM_ERROR after dp->plat_data->attach(),
> maybe I should paste the change that rebase on this patch,
> here are they,
>
> /*
> * This should be the end of attach function, caused
> * we should ensure dp bridge could attach first.
> */
> - if (dp->plat_data && dp->plat_data->attach) {
> + if (dp->plat_data && dp->plat_data->attach)
> ret = dp->plat_data->attach(dp->plat_data, bridge);
> - if (ret) {
> - DRM_ERROR("Failed at platform attch func\n");
> - return ret;
> - }
> - }
>
> - return 0;
> + return ret;
>
>
> If this haven't meet your comment, I maybe start to think that
> your comment "Two new error paths appeared here and above"
> indicated that those two function is the same.
> "dp->plat_data->attach(dp->plat_data, bridge); "
> "drm_panel_attach(dp->plat_data->panel, &dp->connector); "
I wasn't talking about error message but rather about possible need of
clean up in error path. Previously there was only drm_panel_attach().
Now you have two of them (drm_panel_attach() and
dp->plat_data->attach()). If the second fails don't you have to clean up
before exit? I don't know, just asking.
Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2015-09-07 04:30 +0200 |
| Subject | Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir |
| Message-ID | <q5UlY-wp-3@gated-at.bofh.it> |
| In reply to | #1219883 |
Hi Krzysztof,
在 09/07/2015 08:22 AM, Krzysztof Kozlowski 写道:
> On 06.09.2015 16:49, Yakir Yang wrote:
>> Hi Krzysztof,
>>
>> 在 09/04/2015 08:41 AM, Krzysztof Kozlowski 写道:
>>> On 03.09.2015 14:30, Yakir Yang wrote:
>>>> Hi Krzysztof,
>>>>
>>>> 在 09/03/2015 08:58 AM, Krzysztof Kozlowski 写道:
>>>>> On 01.09.2015 14:49, Yakir Yang wrote:
>>>>>> Split the dp core driver from exynos directory to bridge
>>>>>> directory, and rename the core driver to analogix_dp_*,
>>>>>> leave the platform code to analogix_dp-exynos.
>>>>>>
>>>>>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>>>>>> ---
>>>>>> Changes in v4:
>>>>>> - Take Rob suggest, update "analogix,hpd-gpios" to "hpd-gpios" DT
>>>>>> propery.
>>>>>> - Take Jingoo suggest, rename "analogix_dp-exynos.c" file name to
>>>>>> "exynos_dp.c"
>>>>>> - Take Archit suggest, create a separate folder for analogix code in
>>>>>> bridge/
>>>>>>
>>>>>> Changes in v3:
>>>>>> - Take Thierry Reding suggest, move exynos's video_timing code
>>>>>> to analogix_dp-exynos platform driver, add get_modes method
>>>>>> to struct analogix_dp_plat_data.
>>>>>> - Take Heiko suggest, rename some "samsung*" dts propery to
>>>>>> "analogix*".
>>>>>>
>>>>>> Changes in v2:
>>>>>> - Take Jingoo Han suggest, remove new copyright
>>>>>> - Fix compiled failed dut to analogix_dp_device misspell
>> [.....]
>>
>>>>>> -static int exynos_dp_bridge_attach(struct drm_bridge *bridge)
>>>>>> +static int analogix_dp_bridge_attach(struct drm_bridge *bridge)
>>>>>> {
>>>>>> - struct exynos_dp_device *dp = bridge->driver_private;
>>>>>> - struct drm_encoder *encoder = &dp->encoder;
>>>>>> + struct analogix_dp_device *dp = bridge->driver_private;
>>>>>> + struct drm_encoder *encoder = dp->encoder;
>>>>>> struct drm_connector *connector = &dp->connector;
>>>>>> int ret;
>>>>>> - /* Pre-empt DP connector creation if there's a bridge */
>>>>>> - if (dp->ptn_bridge) {
>>>>>> - ret = exynos_drm_attach_lcd_bridge(dp, encoder);
>>>>>> - if (!ret)
>>>>>> - return 0;
>>>>>> + if (!bridge->encoder) {
>>>>>> + DRM_ERROR("Parent encoder object not found");
>>>>>> + return -ENODEV;
>>>>>> }
>>>>>> + encoder->bridge = bridge;
>>>>>> +
>>>>>> connector->polled = DRM_CONNECTOR_POLL_HPD;
>>>>>> ret = drm_connector_init(dp->drm_dev, connector,
>>>>>> - &exynos_dp_connector_funcs,
>>>>>> + &analogix_dp_connector_funcs,
>>>>>> DRM_MODE_CONNECTOR_eDP);
>>>>>> if (ret) {
>>>>>> DRM_ERROR("Failed to initialize connector with drm\n");
>>>>>> return ret;
>>>>>> }
>>>>>> - drm_connector_helper_add(connector,
>>>>>> &exynos_dp_connector_helper_funcs);
>>>>>> + drm_connector_helper_add(connector,
>>>>>> + &analogix_dp_connector_helper_funcs);
>>>>>> drm_connector_register(connector);
>>>>>> drm_mode_connector_attach_encoder(connector, encoder);
>>>>>> - if (dp->panel)
>>>>>> - ret = drm_panel_attach(dp->panel, &dp->connector);
>>>>>> + if (dp->plat_data && dp->plat_data->panel) {
>>>>>> + ret = drm_panel_attach(dp->plat_data->panel, &dp->connector);
>>>>>> + if (ret) {
>>>>>> + DRM_ERROR("Failed to attach panel\n");
>>>>>> + return ret;
>>>>>> + }
>>>>>> + }
>>>>>> +
>>>>>> + /*
>>>>>> + * This should be the end of attach function, caused
>>>>>> + * we should ensure dp bridge could attach first.
>>>>>> + */
>>>>>> + if (dp->plat_data && dp->plat_data->attach) {
>>>>>> + ret = dp->plat_data->attach(dp->plat_data, bridge);
>>>>>> + if (ret) {
>>>>>> + DRM_ERROR("Failed at platform attch func\n");
>>>>> Two new error paths appeared here and above. Don't you have to
>>>>> cleanup something? I don't know, just wondering...
>>>> Hmm... I think both panel & platform_attch need ERROR remind when
>>>> it failed. But if it still need clean, I though it should clean the
>>>> platform attch
>>>> error,
>>>> this is not relate to DRM directly, just analogix driver logic, so
>>>> code would like,
>>>>
>>>> - if (dp->panel)
>>>> - ret = drm_panel_attach(dp->panel, &dp->connector);
>>>> + if (dp->plat_data && dp->plat_data->panel) {
>>>> + ret = drm_panel_attach(dp->plat_data->panel, &dp->connector);
>>>> + if (ret) {
>>>> + DRM_ERROR("Failed to attach panel\n");
>>>> + return ret;
>>>> + }
>>>> + }
>>>>
>>>> + /*
>>>> + * This should be the end of attach function, caused
>>>> + * we should ensure dp bridge could attach first.
>>>> + */
>>>> + if (dp->plat_data && dp->plat_data->attach) {
>>>> + ret = dp->plat_data->attach(dp->plat_data, bridge);
>>>>
>>>> return ret;
>>> I am lost... the code looks the same. What did you change?
>> I just remove the DRM_ERROR after dp->plat_data->attach(),
>> maybe I should paste the change that rebase on this patch,
>> here are they,
>>
>> /*
>> * This should be the end of attach function, caused
>> * we should ensure dp bridge could attach first.
>> */
>> - if (dp->plat_data && dp->plat_data->attach) {
>> + if (dp->plat_data && dp->plat_data->attach)
>> ret = dp->plat_data->attach(dp->plat_data, bridge);
>> - if (ret) {
>> - DRM_ERROR("Failed at platform attch func\n");
>> - return ret;
>> - }
>> - }
>>
>> - return 0;
>> + return ret;
>>
>>
>> If this haven't meet your comment, I maybe start to think that
>> your comment "Two new error paths appeared here and above"
>> indicated that those two function is the same.
>> "dp->plat_data->attach(dp->plat_data, bridge); "
>> "drm_panel_attach(dp->plat_data->panel, &dp->connector); "
> I wasn't talking about error message but rather about possible need of
> clean up in error path. Previously there was only drm_panel_attach().
> Now you have two of them (drm_panel_attach() and
> dp->plat_data->attach()). If the second fails don't you have to clean up
> before exit? I don't know, just asking.
Oh... Sorry about missing your words. Yes,
drm_panel_detach(dp->plat_data->panel)
was missed in this error path.
Thanks,
- Yakir
>
> Best regards,
> Krzysztof
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2015-09-22 09:30 +0200 |
| Subject | [PATCH v5 0/17] Add Analogix Core Display Port Driver |
| Message-ID | <qbqbv-6ZU-1@gated-at.bofh.it> |
| In reply to | #1216602 |
Hi all,
The Samsung Exynos eDP controller and Rockchip RK3288 eDP controller
share the same IP, so a lot of parts can be re-used. I split the common
code into bridge directory, then rk3288 and exynos only need to keep
some platform code. Cause I can't find the exact IP name of exynos dp
controller, so I decide to name dp core driver with "analogix" which I
find in rk3288 eDP TRM :)
This time I create this version on linux-next branch (tag is next-20150918),
and also applied this version to Heiko github eDP branch to verify the function.
(https://github.com/mmind/linux-rockchip/tree/tmp/edp-with-veyron)
Glad to say my chromebook "cnm,n116bgeea2" eDP panel just lighted rightly on
Heiko branch. And after back port this series to chromeos-3.14 tree, my rk3288
SDK board still can light my 2K DisplayPort monitor. So this time would be okay
on mainline kernel and chromeos-3.14 tree. ;)
Due to no Exynos board in my side, so I haven't verified the eDP function on
samsung platform, I only ensure that there are no obvious compiled error. Any
help would be greatly appreciated. :)
Thanks,
- Yakir
Changes in v5:
- Correct the check condition of gpio_is_valid when driver try to get
the "hpd-gpios" DT propery. (Heiko)
- Move the platform attach callback in the front of core driver bridge
attch function. Cause once platform failed at attach, core driver should
still failed, so no need to init connector before platform attached (Krzysztof)
- Keep code style no changes with the previous exynos_dp_code.c in this
patch, and update commit message about the new export symbol (Krzysztof)
- Gather the device type patch (v4 11/16) into this one. (Krzysztof)
- leave out the connector registration to analogix platform driver. (Thierry)
- Resequence this patch after analogix_dp driver have been split
from exynos_dp code, and rephrase reasonable commit message, and
remove some controversial style (Krzysztof)
- analogix_dp_write_byte_to_dpcd(
- dp, DP_TEST_RESPONSE,
+ analogix_dp_write_byte_to_dpcd(dp,
+ DP_TEST_RESPONSE,
DP_TEST_EDID_CHECKSUM_WRITE);
- Switch video timing type to "u32", so driver could use "of_property_read_u32"
to get the backword timing values. Krzysztof suggest me that driver could use
the "of_property_read_bool" to get backword timing values, but that interfacs
would modify the original drm_display_mode timing directly (whether those
properties exists or not).
- Correct the misspell in commit message. (Krzysztof)
- Remove the empty line at the end of document, and correct the endpoint
numbers in the example DT node, and remove the regulator iomux setting
in driver code while using the pinctl in devicetree instead. (Heiko)
- Add device type declared, cause the previous "platform device type
support (v4 11/16)" already merge into (v5 02/14).
- Implement connector registration code. (Thierry)
- Split binding doc's from driver changes. (Rob)
- Add eDP hotplug pinctrl property. (Heiko)
- Remove "reg" DT property, cause driver could poweron/poweroff phy via
the exist "grf" syscon already. And rename the example DT node from
"edp_phy: phy@ff770274" to "edp_phy: edp-phy" directly. (Heiko)
- Add deivce_node at the front of driver, update phy_ops type from "static
struct" to "static const struct". And correct the input paramters of
devm_phy_create() interfaces. (Heiko)
- Split binding doc's from driver changes. (Rob)
- Update the rockchip,grf explain in document, and correct the clock required
elemets in document. (Rob & Heiko)
- Fix compiled error (Heiko)
- Using the connector display info message to configure eDP driver input
video mode, but hard code CRTC video output mode to RGBaaa.
Changes in v4:
- Update "analogix,hpd-gpios" to "hpd-gpios" DT propery. (Rob)
- Rename "analogix_dp-exynos.c" file name to "exynos_dp.c" (Jingoo)
- Create a separate folder for analogix code in bridge/ (Archit)
- Update commit message more readable. (Jingoo)
- Adjust the order from 05 to 04
- Provide backword compatibility with samsung. (Krzysztof)
- Split all DTS changes, and provide backward compatibility. Mark old
properties as deprecated but still support them. (Krzysztof)
- Update "analogix,hpd-gpio" to "hpd-gpios" prop name. (Rob)
- Deprecated some properties which could parsed from Edid/Mode/DPCD. (Thierry)
"analogix,color-space" & "analogix,color-depth" &
"analogix,link-rate" & "analogix,lane-count" &
"analogix,ycbcr-coeff" & "analogix,dynamic-range" &
"vsync-active-high" & "hsync-active-high" & "interlaces"
- Separate all DTS changes to a separate patch. (Krzysztof)
- Remove some deprecated DT properties in rockchip dp document.
- Add commit message, and remove the redundant rockchip_dp_phy_init()
function, move those code to probe() method. And remove driver .owner
number. (Kishon)
- Seprate the link-rate and lane-count limit out with the device_type
flag. (Thierry)
- Take Jingoo suggest, add commit messages.
- Call drm_panel_prepare() in .get_modes function, ensure panel should
power on before driver try to read edid message.
Changes in v3:
- Move exynos's video_timing code to analogix_dp-exynos platform driver,
add get_modes method to struct analogix_dp_plat_data. (Thierry)
- Rename some "samsung*" dts propery to "analogix*". (Heiko)
- The link_rate and lane_count shouldn't config to the DT property value
directly, but we can take those as hardware limite. For example, RK3288
only support 4 physical lanes of 2.7/1.62 Gbps/lane, so DT property would
like "link-rate = 0x0a" "lane-count = 4". (Thierry)
- Dynamic parse video timing info from struct drm_display_mode and
struct drm_display_info. (Thierry)
- Add devicetree binding documents. (Heiko)
- Remove sync pol & colorimetry properies from the new analogix dp driver
devicetree binding. (Thierry)
- Update the exist exynos dtsi file with the latest DP DT properies.
- Leave "sclk_edp_24m" to rockchip dp phy driver which name to "24m",
and leave "sclk_edp" to analogix dp core driver which name to "dp",
and leave "pclk_edp" to rockchip dp platform driver which name to
"pclk". (Thierry & Heiko)
- Add devicetree binding document. (Heiko)
- Remove "rockchip,panel" DT property, take use of remote point to get panel
node. (Heiko)
- Add the new function point dp_platdata->get_modes() init.
- Suggest, add rockchip dp phy driver, collect the phy clocks and
power control. (Heiko)
- Add "analogix,need-force-hpd" to indicate whether driver need foce
hpd when hpd detect failed.
- move dp hpd detect to connector detect function.
- Add edid modes parse support
Changes in v2:
- Keep author name list no changed (Jingoo)
- Remove new copyright (Jingoo)
- Fix compiled failed due to analogix_dp_device misspell
- Improved commit message more readable, and avoid using some
uncommon style like bellow: (Joe Preches)
- retval = exynos_dp_read_bytes_from_i2c(...
...);
+ retval =
+ exynos_dp_read_bytes_from_i2c(......);
- Get panel node with remote-endpoint method, and create devicetree binding
for driver. (Heiko)
- Remove the clock enable/disbale with "sclk_edp" & "sclk_edp_24m",
leave those clock to rockchip dp phy driver.
- Fix compile failed dut to phy_pd_addr variable misspell error
Mark Yao (1):
drm: rockchip: vop: add bpc and color mode setting
Yakir Yang (16):
drm: exynos: dp: convert to drm bridge mode
drm: bridge: analogix/dp: split exynos dp driver to bridge directory
drm: bridge: analogix/dp: fix some obvious code style
drm: bridge: analogix/dp: remove duplicate configuration of link rate
and link count
drm: bridge: analogix/dp: dynamic parse sync_pol & interlace &
dynamic_range
Documentation: drm/bridge: add document for analogix_dp
ARM: dts: exynos/dp: remove some properties that deprecated by
analogix_dp driver
drm: rockchip: dp: add rockchip platform dp driver
Documentation: drm/bridge: add document for analogix_dp
phy: Add driver for rockchip Display Port PHY
Documentation: phy: add document for rockchip dp phy
drm: bridge: analogix/dp: add some rk3288 special registers setting
drm: bridge: analogix/dp: add max link rate and lane count limit for
RK3288
drm: bridge: analogix/dp: try force hpd after plug in lookup failed
drm: bridge: analogix/dp: move hpd detect to connector detect function
drm: bridge: analogix/dp: add edid modes parse in get_modes method
.../devicetree/bindings/drm/bridge/analogix_dp.txt | 52 +
.../devicetree/bindings/phy/rockchip-dp-phy.txt | 22 +
.../bindings/video/analogix_dp-rockchip.txt | 91 ++
.../devicetree/bindings/video/exynos_dp.txt | 64 +-
arch/arm/boot/dts/exynos5250-arndale.dts | 2 -
arch/arm/boot/dts/exynos5250-smdk5250.dts | 2 -
arch/arm/boot/dts/exynos5250-snow.dts | 4 +-
arch/arm/boot/dts/exynos5250-spring.dts | 4 +-
arch/arm/boot/dts/exynos5420-peach-pit.dts | 4 +-
arch/arm/boot/dts/exynos5420-smdk5420.dts | 2 -
arch/arm/boot/dts/exynos5800-peach-pi.dts | 4 +-
drivers/gpu/drm/bridge/Kconfig | 2 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/analogix/Kconfig | 4 +
drivers/gpu/drm/bridge/analogix/Makefile | 1 +
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 1372 +++++++++++++++++++
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 286 ++++
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 1294 ++++++++++++++++++
.../analogix/analogix_dp_reg.h} | 270 ++--
drivers/gpu/drm/exynos/Kconfig | 3 +-
drivers/gpu/drm/exynos/Makefile | 2 +-
drivers/gpu/drm/exynos/exynos_dp.c | 366 +++++
drivers/gpu/drm/exynos/exynos_dp_core.c | 1427 --------------------
drivers/gpu/drm/exynos/exynos_dp_core.h | 281 ----
drivers/gpu/drm/exynos/exynos_dp_reg.c | 1263 -----------------
drivers/gpu/drm/rockchip/Kconfig | 9 +
drivers/gpu/drm/rockchip/Makefile | 1 +
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 457 +++++++
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 2 +-
drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 +-
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 32 +-
drivers/phy/Kconfig | 7 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-rockchip-dp.c | 151 +++
include/drm/bridge/analogix_dp.h | 42 +
35 files changed, 4360 insertions(+), 3167 deletions(-)
create mode 100644 Documentation/devicetree/bindings/drm/bridge/analogix_dp.txt
create mode 100644 Documentation/devicetree/bindings/phy/rockchip-dp-phy.txt
create mode 100644 Documentation/devicetree/bindings/video/analogix_dp-rockchip.txt
create mode 100644 drivers/gpu/drm/bridge/analogix/Kconfig
create mode 100644 drivers/gpu/drm/bridge/analogix/Makefile
create mode 100644 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
create mode 100644 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
create mode 100644 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
rename drivers/gpu/drm/{exynos/exynos_dp_reg.h => bridge/analogix/analogix_dp_reg.h} (62%)
create mode 100644 drivers/gpu/drm/exynos/exynos_dp.c
delete mode 100644 drivers/gpu/drm/exynos/exynos_dp_core.c
delete mode 100644 drivers/gpu/drm/exynos/exynos_dp_core.h
delete mode 100644 drivers/gpu/drm/exynos/exynos_dp_reg.c
create mode 100644 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
create mode 100644 drivers/phy/phy-rockchip-dp.c
create mode 100644 include/drm/bridge/analogix_dp.h
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2015-09-22 09:30 +0200 |
| Subject | [PATCH v5 01/17] drm: exynos: dp: convert to drm bridge mode |
| Message-ID | <qbqbv-6ZU-5@gated-at.bofh.it> |
| In reply to | #1229939 |
In order to move exynos dp code to bridge directory,
we need to convert driver drm bridge mode first. As
dp driver already have a ptn3460 bridge, so we need
to move ptn bridge to the next bridge of dp bridge.
Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
- Keep author name list no changed (Jingoo)
drivers/gpu/drm/exynos/exynos_dp_core.c | 104 +++++++++++++++++++++++---------
drivers/gpu/drm/exynos/exynos_dp_core.h | 1 +
2 files changed, 78 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
index d66ade0..e0818c1 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -1009,9 +1009,9 @@ static int exynos_drm_attach_lcd_bridge(struct exynos_dp_device *dp,
{
int ret;
- encoder->bridge = dp->bridge;
- dp->bridge->encoder = encoder;
- ret = drm_bridge_attach(encoder->dev, dp->bridge);
+ encoder->bridge->next = dp->ptn_bridge;
+ dp->ptn_bridge->encoder = encoder;
+ ret = drm_bridge_attach(encoder->dev, dp->ptn_bridge);
if (ret) {
DRM_ERROR("Failed to attach bridge to drm\n");
return ret;
@@ -1020,14 +1020,15 @@ static int exynos_drm_attach_lcd_bridge(struct exynos_dp_device *dp,
return 0;
}
-static int exynos_dp_create_connector(struct drm_encoder *encoder)
+static int exynos_dp_bridge_attach(struct drm_bridge *bridge)
{
- struct exynos_dp_device *dp = encoder_to_dp(encoder);
+ struct exynos_dp_device *dp = bridge->driver_private;
+ struct drm_encoder *encoder = &dp->encoder;
struct drm_connector *connector = &dp->connector;
int ret;
/* Pre-empt DP connector creation if there's a bridge */
- if (dp->bridge) {
+ if (dp->ptn_bridge) {
ret = exynos_drm_attach_lcd_bridge(dp, encoder);
if (!ret)
return 0;
@@ -1052,22 +1053,9 @@ static int exynos_dp_create_connector(struct drm_encoder *encoder)
return ret;
}
-static bool exynos_dp_mode_fixup(struct drm_encoder *encoder,
- const struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
-{
- return true;
-}
-
-static void exynos_dp_mode_set(struct drm_encoder *encoder,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
-{
-}
-
-static void exynos_dp_enable(struct drm_encoder *encoder)
+static void exynos_dp_bridge_enable(struct drm_bridge *bridge)
{
- struct exynos_dp_device *dp = encoder_to_dp(encoder);
+ struct exynos_dp_device *dp = bridge->driver_private;
struct exynos_drm_crtc *crtc = dp_to_crtc(dp);
if (dp->dpms_mode == DRM_MODE_DPMS_ON)
@@ -1092,9 +1080,9 @@ static void exynos_dp_enable(struct drm_encoder *encoder)
dp->dpms_mode = DRM_MODE_DPMS_ON;
}
-static void exynos_dp_disable(struct drm_encoder *encoder)
+static void exynos_dp_bridge_disable(struct drm_bridge *bridge)
{
- struct exynos_dp_device *dp = encoder_to_dp(encoder);
+ struct exynos_dp_device *dp = bridge->driver_private;
struct exynos_drm_crtc *crtc = dp_to_crtc(dp);
if (dp->dpms_mode != DRM_MODE_DPMS_ON)
@@ -1123,6 +1111,68 @@ static void exynos_dp_disable(struct drm_encoder *encoder)
dp->dpms_mode = DRM_MODE_DPMS_OFF;
}
+static void exynos_dp_bridge_nop(struct drm_bridge *bridge)
+{
+ /* do nothing */
+}
+
+static const struct drm_bridge_funcs exynos_dp_bridge_funcs = {
+ .enable = exynos_dp_bridge_enable,
+ .disable = exynos_dp_bridge_disable,
+ .pre_enable = exynos_dp_bridge_nop,
+ .post_disable = exynos_dp_bridge_nop,
+ .attach = exynos_dp_bridge_attach,
+};
+
+static int exynos_dp_create_connector(struct drm_encoder *encoder)
+{
+ struct exynos_dp_device *dp = encoder_to_dp(encoder);
+ struct drm_device *drm_dev = dp->drm_dev;
+ struct drm_bridge *bridge;
+ int ret;
+
+ bridge = devm_kzalloc(drm_dev->dev, sizeof(*bridge), GFP_KERNEL);
+ if (!bridge) {
+ DRM_ERROR("failed to allocate for drm bridge\n");
+ return -ENOMEM;
+ }
+
+ dp->bridge = bridge;
+
+ bridge->driver_private = dp;
+ bridge->encoder = encoder;
+ bridge->funcs = &exynos_dp_bridge_funcs;
+
+ ret = drm_bridge_attach(drm_dev, bridge);
+ if (ret) {
+ DRM_ERROR("failed to attach drm bridge\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static bool exynos_dp_mode_fixup(struct drm_encoder *encoder,
+ const struct drm_display_mode *mode,
+ struct drm_display_mode *adjusted_mode)
+{
+ return true;
+}
+
+static void exynos_dp_mode_set(struct drm_encoder *encoder,
+ struct drm_display_mode *mode,
+ struct drm_display_mode *adjusted_mode)
+{
+}
+
+static void exynos_dp_enable(struct drm_encoder *encoder)
+{
+}
+
+static void exynos_dp_disable(struct drm_encoder *encoder)
+{
+}
+
static struct drm_encoder_helper_funcs exynos_dp_encoder_helper_funcs = {
.mode_fixup = exynos_dp_mode_fixup,
.mode_set = exynos_dp_mode_set,
@@ -1365,9 +1415,9 @@ static int exynos_dp_probe(struct platform_device *pdev)
if (endpoint) {
bridge_node = of_graph_get_remote_port_parent(endpoint);
if (bridge_node) {
- dp->bridge = of_drm_find_bridge(bridge_node);
+ dp->ptn_bridge = of_drm_find_bridge(bridge_node);
of_node_put(bridge_node);
- if (!dp->bridge)
+ if (!dp->ptn_bridge)
return -EPROBE_DEFER;
} else
return -EPROBE_DEFER;
@@ -1388,7 +1438,7 @@ static int exynos_dp_suspend(struct device *dev)
{
struct exynos_dp_device *dp = dev_get_drvdata(dev);
- exynos_dp_disable(&dp->encoder);
+ exynos_dp_bridge_disable(dp->bridge);
return 0;
}
@@ -1396,7 +1446,7 @@ static int exynos_dp_resume(struct device *dev)
{
struct exynos_dp_device *dp = dev_get_drvdata(dev);
- exynos_dp_enable(&dp->encoder);
+ exynos_dp_bridge_enable(dp->bridge);
return 0;
}
#endif
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.h b/drivers/gpu/drm/exynos/exynos_dp_core.h
index e413b6f..66eec4b 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.h
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.h
@@ -153,6 +153,7 @@ struct exynos_dp_device {
struct drm_connector connector;
struct drm_panel *panel;
struct drm_bridge *bridge;
+ struct drm_bridge *ptn_bridge;
struct clk *clock;
unsigned int irq;
void __iomem *reg_base;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2015-09-22 09:40 +0200 |
| Subject | [PATCH v5 03/17] drm: bridge: analogix/dp: fix some obvious code style |
| Message-ID | <qbqlc-7b4-1@gated-at.bofh.it> |
| In reply to | #1229939 |
Fix some obvious alignment problems, like alignment and line
over 80 characters problems, make this easy to be maintained
later.
Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v5:
- Resequence this patch after analogix_dp driver have been split
from exynos_dp code, and rephrase reasonable commit message, and
remove some controversial style (Krzysztof)
- analogix_dp_write_byte_to_dpcd(
- dp, DP_TEST_RESPONSE,
+ analogix_dp_write_byte_to_dpcd(dp,
+ DP_TEST_RESPONSE,
DP_TEST_EDID_CHECKSUM_WRITE);
Changes in v4: None
Changes in v3: None
Changes in v2:
- Improved commit message more readable, and avoid using some
uncommon style like bellow: (Joe Preches)
- retval = exynos_dp_read_bytes_from_i2c(...
...);
+ retval =
+ exynos_dp_read_bytes_from_i2c(......);
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 129 ++++++++++-----------
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 72 ++++++------
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 124 ++++++++++----------
3 files changed, 163 insertions(+), 162 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 8a7ba12..fa9eb19 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -61,7 +61,7 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
while (analogix_dp_get_plug_in_status(dp) != 0) {
timeout_loop++;
- if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
+ if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
dev_err(dp->dev, "failed to get hpd plug status\n");
return -ETIMEDOUT;
}
@@ -98,8 +98,8 @@ static int analogix_dp_read_edid(struct analogix_dp_device *dp)
/* Read Extension Flag, Number of 128-byte EDID extension blocks */
retval = analogix_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
- EDID_EXTENSION_FLAG,
- &extend_block);
+ EDID_EXTENSION_FLAG,
+ &extend_block);
if (retval)
return retval;
@@ -107,7 +107,8 @@ static int analogix_dp_read_edid(struct analogix_dp_device *dp)
dev_dbg(dp->dev, "EDID data includes a single extension!\n");
/* Read EDID data */
- retval = analogix_dp_read_bytes_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
+ retval = analogix_dp_read_bytes_from_i2c(dp,
+ I2C_EDID_DEVICE_ADDR,
EDID_HEADER_PATTERN,
EDID_BLOCK_LENGTH,
&edid[EDID_HEADER_PATTERN]);
@@ -138,7 +139,7 @@ static int analogix_dp_read_edid(struct analogix_dp_device *dp)
}
analogix_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST,
- &test_vector);
+ &test_vector);
if (test_vector & DP_TEST_LINK_EDID_READ) {
analogix_dp_write_byte_to_dpcd(dp,
DP_TEST_EDID_CHECKSUM,
@@ -152,10 +153,8 @@ static int analogix_dp_read_edid(struct analogix_dp_device *dp)
/* Read EDID data */
retval = analogix_dp_read_bytes_from_i2c(dp,
- I2C_EDID_DEVICE_ADDR,
- EDID_HEADER_PATTERN,
- EDID_BLOCK_LENGTH,
- &edid[EDID_HEADER_PATTERN]);
+ I2C_EDID_DEVICE_ADDR, EDID_HEADER_PATTERN,
+ EDID_BLOCK_LENGTH, &edid[EDID_HEADER_PATTERN]);
if (retval != 0) {
dev_err(dp->dev, "EDID Read failed!\n");
return -EIO;
@@ -166,16 +165,13 @@ static int analogix_dp_read_edid(struct analogix_dp_device *dp)
return -EIO;
}
- analogix_dp_read_byte_from_dpcd(dp,
- DP_TEST_REQUEST,
- &test_vector);
+ analogix_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST,
+ &test_vector);
if (test_vector & DP_TEST_LINK_EDID_READ) {
analogix_dp_write_byte_to_dpcd(dp,
- DP_TEST_EDID_CHECKSUM,
- edid[EDID_CHECKSUM]);
+ DP_TEST_EDID_CHECKSUM, edid[EDID_CHECKSUM]);
analogix_dp_write_byte_to_dpcd(dp,
- DP_TEST_RESPONSE,
- DP_TEST_EDID_CHECKSUM_WRITE);
+ DP_TEST_RESPONSE, DP_TEST_EDID_CHECKSUM_WRITE);
}
}
@@ -190,8 +186,7 @@ static int analogix_dp_handle_edid(struct analogix_dp_device *dp)
int retval;
/* Read DPCD DP_DPCD_REV~RECEIVE_PORT1_CAP_1 */
- retval = analogix_dp_read_bytes_from_dpcd(dp, DP_DPCD_REV,
- 12, buf);
+ retval = analogix_dp_read_bytes_from_dpcd(dp, DP_DPCD_REV, 12, buf);
if (retval)
return retval;
@@ -205,8 +200,9 @@ static int analogix_dp_handle_edid(struct analogix_dp_device *dp)
return retval;
}
-static void analogix_dp_enable_rx_to_enhanced_mode(struct analogix_dp_device *dp,
- bool enable)
+static void
+analogix_dp_enable_rx_to_enhanced_mode(struct analogix_dp_device *dp,
+ bool enable)
{
u8 data;
@@ -214,11 +210,11 @@ static void analogix_dp_enable_rx_to_enhanced_mode(struct analogix_dp_device *dp
if (enable)
analogix_dp_write_byte_to_dpcd(dp, DP_LANE_COUNT_SET,
- DP_LANE_COUNT_ENHANCED_FRAME_EN |
- DPCD_LANE_COUNT_SET(data));
+ DP_LANE_COUNT_ENHANCED_FRAME_EN |
+ DPCD_LANE_COUNT_SET(data));
else
analogix_dp_write_byte_to_dpcd(dp, DP_LANE_COUNT_SET,
- DPCD_LANE_COUNT_SET(data));
+ DPCD_LANE_COUNT_SET(data));
}
static int analogix_dp_is_enhanced_mode_available(struct analogix_dp_device *dp)
@@ -245,13 +241,13 @@ static void analogix_dp_training_pattern_dis(struct analogix_dp_device *dp)
{
analogix_dp_set_training_pattern(dp, DP_NONE);
- analogix_dp_write_byte_to_dpcd(dp,
- DP_TRAINING_PATTERN_SET,
- DP_TRAINING_PATTERN_DISABLE);
+ analogix_dp_write_byte_to_dpcd(dp, DP_TRAINING_PATTERN_SET,
+ DP_TRAINING_PATTERN_DISABLE);
}
-static void analogix_dp_set_lane_lane_pre_emphasis(struct analogix_dp_device *dp,
- int pre_emphasis, int lane)
+static void
+analogix_dp_set_lane_lane_pre_emphasis(struct analogix_dp_device *dp,
+ int pre_emphasis, int lane)
{
switch (lane) {
case 0:
@@ -291,8 +287,7 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp)
/* Setup RX configuration */
buf[0] = dp->link_train.link_rate;
buf[1] = dp->link_train.lane_count;
- retval = analogix_dp_write_bytes_to_dpcd(dp, DP_LINK_BW_SET,
- 2, buf);
+ retval = analogix_dp_write_bytes_to_dpcd(dp, DP_LINK_BW_SET, 2, buf);
if (retval)
return retval;
@@ -328,7 +323,7 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp)
DP_TRAIN_VOLTAGE_SWING_LEVEL_0;
retval = analogix_dp_write_bytes_to_dpcd(dp, DP_TRAINING_LANE0_SET,
- lane_count, buf);
+ lane_count, buf);
return retval;
}
@@ -336,7 +331,7 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp)
static unsigned char analogix_dp_get_lane_status(u8 link_status[2], int lane)
{
int shift = (lane & 1) * 4;
- u8 link_value = link_status[lane>>1];
+ u8 link_value = link_status[lane >> 1];
return (link_value >> shift) & 0xf;
}
@@ -355,7 +350,7 @@ static int analogix_dp_clock_recovery_ok(u8 link_status[2], int lane_count)
}
static int analogix_dp_channel_eq_ok(u8 link_status[2], u8 link_align,
- int lane_count)
+ int lane_count)
{
int lane;
u8 lane_status;
@@ -373,11 +368,11 @@ static int analogix_dp_channel_eq_ok(u8 link_status[2], u8 link_align,
return 0;
}
-static unsigned char analogix_dp_get_adjust_request_voltage(u8 adjust_request[2],
- int lane)
+static unsigned char
+analogix_dp_get_adjust_request_voltage(u8 adjust_request[2], int lane)
{
int shift = (lane & 1) * 4;
- u8 link_value = adjust_request[lane>>1];
+ u8 link_value = adjust_request[lane >> 1];
return (link_value >> shift) & 0x3;
}
@@ -387,13 +382,13 @@ static unsigned char analogix_dp_get_adjust_request_pre_emphasis(
int lane)
{
int shift = (lane & 1) * 4;
- u8 link_value = adjust_request[lane>>1];
+ u8 link_value = adjust_request[lane >> 1];
return ((link_value >> shift) & 0xc) >> 2;
}
static void analogix_dp_set_lane_link_training(struct analogix_dp_device *dp,
- u8 training_lane_set, int lane)
+ u8 training_lane_set, int lane)
{
switch (lane) {
case 0:
@@ -413,9 +408,9 @@ static void analogix_dp_set_lane_link_training(struct analogix_dp_device *dp,
}
}
-static unsigned int analogix_dp_get_lane_link_training(
- struct analogix_dp_device *dp,
- int lane)
+static unsigned int
+analogix_dp_get_lane_link_training(struct analogix_dp_device *dp,
+ int lane)
{
u32 reg;
@@ -449,7 +444,7 @@ static void analogix_dp_reduce_link_rate(struct analogix_dp_device *dp)
}
static void analogix_dp_get_adjust_training_lane(struct analogix_dp_device *dp,
- u8 adjust_request[2])
+ u8 adjust_request[2])
{
int lane, lane_count;
u8 voltage_swing, pre_emphasis, training_lane;
@@ -622,7 +617,7 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
}
static void analogix_dp_get_max_rx_bandwidth(struct analogix_dp_device *dp,
- u8 *bandwidth)
+ u8 *bandwidth)
{
u8 data;
@@ -635,7 +630,7 @@ static void analogix_dp_get_max_rx_bandwidth(struct analogix_dp_device *dp,
}
static void analogix_dp_get_max_rx_lane_count(struct analogix_dp_device *dp,
- u8 *lane_count)
+ u8 *lane_count)
{
u8 data;
@@ -648,8 +643,8 @@ static void analogix_dp_get_max_rx_lane_count(struct analogix_dp_device *dp,
}
static void analogix_dp_init_training(struct analogix_dp_device *dp,
- enum link_lane_count_type max_lane,
- enum link_rate_type max_rate)
+ enum link_lane_count_type max_lane,
+ enum link_rate_type max_rate)
{
/*
* MACRO_RST must be applied after the PLL_LOCK to avoid
@@ -662,7 +657,7 @@ static void analogix_dp_init_training(struct analogix_dp_device *dp,
analogix_dp_get_max_rx_lane_count(dp, &dp->link_train.lane_count);
if ((dp->link_train.link_rate != LINK_RATE_1_62GBPS) &&
- (dp->link_train.link_rate != LINK_RATE_2_70GBPS)) {
+ (dp->link_train.link_rate != LINK_RATE_2_70GBPS)) {
dev_err(dp->dev, "Rx Max Link Rate is abnormal :%x !\n",
dp->link_train.link_rate);
dp->link_train.link_rate = LINK_RATE_1_62GBPS;
@@ -722,8 +717,7 @@ static int analogix_dp_sw_link_training(struct analogix_dp_device *dp)
}
static int analogix_dp_set_link_train(struct analogix_dp_device *dp,
- u32 count,
- u32 bwtype)
+ u32 count, u32 bwtype)
{
int i;
int retval;
@@ -759,7 +753,7 @@ static int analogix_dp_config_video(struct analogix_dp_device *dp)
timeout_loop++;
if (analogix_dp_is_slave_video_stream_clock_on(dp) == 0)
break;
- if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
+ if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
dev_err(dp->dev, "Timeout of video streamclk ok\n");
return -ETIMEDOUT;
}
@@ -790,7 +784,7 @@ static int analogix_dp_config_video(struct analogix_dp_device *dp)
} else if (done_count) {
done_count = 0;
}
- if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
+ if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
dev_err(dp->dev, "Timeout of video streamclk ok\n");
return -ETIMEDOUT;
}
@@ -804,25 +798,24 @@ static int analogix_dp_config_video(struct analogix_dp_device *dp)
return retval;
}
-static void analogix_dp_enable_scramble(struct analogix_dp_device *dp, bool enable)
+static void analogix_dp_enable_scramble(struct analogix_dp_device *dp,
+ bool enable)
{
u8 data;
if (enable) {
analogix_dp_enable_scrambling(dp);
- analogix_dp_read_byte_from_dpcd(dp,
- DP_TRAINING_PATTERN_SET,
- &data);
+ analogix_dp_read_byte_from_dpcd(dp, DP_TRAINING_PATTERN_SET,
+ &data);
analogix_dp_write_byte_to_dpcd(dp,
DP_TRAINING_PATTERN_SET,
(u8)(data & ~DP_LINK_SCRAMBLING_DISABLE));
} else {
analogix_dp_disable_scrambling(dp);
- analogix_dp_read_byte_from_dpcd(dp,
- DP_TRAINING_PATTERN_SET,
- &data);
+ analogix_dp_read_byte_from_dpcd(dp, DP_TRAINING_PATTERN_SET,
+ &data);
analogix_dp_write_byte_to_dpcd(dp,
DP_TRAINING_PATTERN_SET,
(u8)(data | DP_LINK_SCRAMBLING_DISABLE));
@@ -895,7 +888,7 @@ static void analogix_dp_commit(struct analogix_dp_device *dp)
}
ret = analogix_dp_set_link_train(dp, dp->video_info->lane_count,
- dp->video_info->link_rate);
+ dp->video_info->link_rate);
if (ret) {
dev_err(dp->dev, "unable to do link train\n");
return;
@@ -1082,8 +1075,8 @@ static struct video_info *analogix_dp_dt_parse_pdata(struct device *dev)
struct device_node *dp_node = dev->of_node;
struct video_info *dp_video_config;
- dp_video_config = devm_kzalloc(dev,
- sizeof(*dp_video_config), GFP_KERNEL);
+ dp_video_config = devm_kzalloc(dev, sizeof(*dp_video_config),
+ GFP_KERNEL);
if (!dp_video_config)
return ERR_PTR(-ENOMEM);
@@ -1097,37 +1090,37 @@ static struct video_info *analogix_dp_dt_parse_pdata(struct device *dev)
of_property_read_bool(dp_node, "interlaced");
if (of_property_read_u32(dp_node, "samsung,color-space",
- &dp_video_config->color_space)) {
+ &dp_video_config->color_space)) {
dev_err(dev, "failed to get color-space\n");
return ERR_PTR(-EINVAL);
}
if (of_property_read_u32(dp_node, "samsung,dynamic-range",
- &dp_video_config->dynamic_range)) {
+ &dp_video_config->dynamic_range)) {
dev_err(dev, "failed to get dynamic-range\n");
return ERR_PTR(-EINVAL);
}
if (of_property_read_u32(dp_node, "samsung,ycbcr-coeff",
- &dp_video_config->ycbcr_coeff)) {
+ &dp_video_config->ycbcr_coeff)) {
dev_err(dev, "failed to get ycbcr-coeff\n");
return ERR_PTR(-EINVAL);
}
if (of_property_read_u32(dp_node, "samsung,color-depth",
- &dp_video_config->color_depth)) {
+ &dp_video_config->color_depth)) {
dev_err(dev, "failed to get color-depth\n");
return ERR_PTR(-EINVAL);
}
if (of_property_read_u32(dp_node, "samsung,link-rate",
- &dp_video_config->link_rate)) {
+ &dp_video_config->link_rate)) {
dev_err(dev, "failed to get link-rate\n");
return ERR_PTR(-EINVAL);
}
if (of_property_read_u32(dp_node, "samsung,lane-count",
- &dp_video_config->lane_count)) {
+ &dp_video_config->lane_count)) {
dev_err(dev, "failed to get lane-count\n");
return ERR_PTR(-EINVAL);
}
@@ -1238,7 +1231,7 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
analogix_dp_init_dp(dp);
ret = devm_request_irq(&pdev->dev, dp->irq, analogix_dp_irq_handler,
- irq_flags, "analogix-dp", dp);
+ irq_flags, "analogix-dp", dp);
if (ret) {
dev_err(&pdev->dev, "failed to request irq\n");
return ret;
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index 65ce7f1..14d20be 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -187,50 +187,55 @@ int analogix_dp_get_plug_in_status(struct analogix_dp_device *dp);
void analogix_dp_enable_sw_function(struct analogix_dp_device *dp);
int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp);
int analogix_dp_write_byte_to_dpcd(struct analogix_dp_device *dp,
- unsigned int reg_addr,
- unsigned char data);
+ unsigned int reg_addr,
+ unsigned char data);
int analogix_dp_read_byte_from_dpcd(struct analogix_dp_device *dp,
- unsigned int reg_addr,
- unsigned char *data);
+ unsigned int reg_addr,
+ unsigned char *data);
int analogix_dp_write_bytes_to_dpcd(struct analogix_dp_device *dp,
- unsigned int reg_addr,
- unsigned int count,
- unsigned char data[]);
+ unsigned int reg_addr,
+ unsigned int count,
+ unsigned char data[]);
int analogix_dp_read_bytes_from_dpcd(struct analogix_dp_device *dp,
- unsigned int reg_addr,
- unsigned int count,
- unsigned char data[]);
+ unsigned int reg_addr,
+ unsigned int count,
+ unsigned char data[]);
int analogix_dp_select_i2c_device(struct analogix_dp_device *dp,
- unsigned int device_addr,
- unsigned int reg_addr);
+ unsigned int device_addr,
+ unsigned int reg_addr);
int analogix_dp_read_byte_from_i2c(struct analogix_dp_device *dp,
- unsigned int device_addr,
- unsigned int reg_addr,
- unsigned int *data);
+ unsigned int device_addr,
+ unsigned int reg_addr,
+ unsigned int *data);
int analogix_dp_read_bytes_from_i2c(struct analogix_dp_device *dp,
- unsigned int device_addr,
- unsigned int reg_addr,
- unsigned int count,
- unsigned char edid[]);
+ unsigned int device_addr,
+ unsigned int reg_addr,
+ unsigned int count,
+ unsigned char edid[]);
void analogix_dp_set_link_bandwidth(struct analogix_dp_device *dp, u32 bwtype);
void analogix_dp_get_link_bandwidth(struct analogix_dp_device *dp, u32 *bwtype);
void analogix_dp_set_lane_count(struct analogix_dp_device *dp, u32 count);
void analogix_dp_get_lane_count(struct analogix_dp_device *dp, u32 *count);
-void analogix_dp_enable_enhanced_mode(struct analogix_dp_device *dp, bool enable);
+void analogix_dp_enable_enhanced_mode(struct analogix_dp_device *dp,
+ bool enable);
void analogix_dp_set_training_pattern(struct analogix_dp_device *dp,
- enum pattern_set pattern);
-void analogix_dp_set_lane0_pre_emphasis(struct analogix_dp_device *dp, u32 level);
-void analogix_dp_set_lane1_pre_emphasis(struct analogix_dp_device *dp, u32 level);
-void analogix_dp_set_lane2_pre_emphasis(struct analogix_dp_device *dp, u32 level);
-void analogix_dp_set_lane3_pre_emphasis(struct analogix_dp_device *dp, u32 level);
+ enum pattern_set pattern);
+void analogix_dp_set_lane0_pre_emphasis(struct analogix_dp_device *dp,
+ u32 level);
+void analogix_dp_set_lane1_pre_emphasis(struct analogix_dp_device *dp,
+ u32 level);
+void analogix_dp_set_lane2_pre_emphasis(struct analogix_dp_device *dp,
+ u32 level);
+void analogix_dp_set_lane3_pre_emphasis(struct analogix_dp_device *dp,
+ u32 level);
void analogix_dp_set_lane0_link_training(struct analogix_dp_device *dp,
- u32 training_lane);
+ u32 training_lane);
void analogix_dp_set_lane1_link_training(struct analogix_dp_device *dp,
- u32 training_lane);
+ u32 training_lane);
void analogix_dp_set_lane2_link_training(struct analogix_dp_device *dp,
- u32 training_lane);
+ u32 training_lane);
void analogix_dp_set_lane3_link_training(struct analogix_dp_device *dp,
- u32 training_lane);
+ u32 training_lane);
u32 analogix_dp_get_lane0_link_training(struct analogix_dp_device *dp);
u32 analogix_dp_get_lane1_link_training(struct analogix_dp_device *dp);
u32 analogix_dp_get_lane2_link_training(struct analogix_dp_device *dp);
@@ -241,11 +246,12 @@ void analogix_dp_init_video(struct analogix_dp_device *dp);
void analogix_dp_set_video_color_format(struct analogix_dp_device *dp);
int analogix_dp_is_slave_video_stream_clock_on(struct analogix_dp_device *dp);
void analogix_dp_set_video_cr_mn(struct analogix_dp_device *dp,
- enum clock_recovery_m_value_type type,
- u32 m_value,
- u32 n_value);
+ enum clock_recovery_m_value_type type,
+ u32 m_value,
+ u32 n_value);
void analogix_dp_set_video_timing_mode(struct analogix_dp_device *dp, u32 type);
-void analogix_dp_enable_video_master(struct analogix_dp_device *dp, bool enable);
+void analogix_dp_enable_video_master(struct analogix_dp_device *dp,
+ bool enable);
void analogix_dp_start_video(struct analogix_dp_device *dp);
int analogix_dp_is_video_stream_on(struct analogix_dp_device *dp);
void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index 442cc66..a388c0a 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -54,10 +54,10 @@ void analogix_dp_lane_swap(struct analogix_dp_device *dp, bool enable)
if (enable)
reg = LANE3_MAP_LOGIC_LANE_0 | LANE2_MAP_LOGIC_LANE_1 |
- LANE1_MAP_LOGIC_LANE_2 | LANE0_MAP_LOGIC_LANE_3;
+ LANE1_MAP_LOGIC_LANE_2 | LANE0_MAP_LOGIC_LANE_3;
else
reg = LANE3_MAP_LOGIC_LANE_3 | LANE2_MAP_LOGIC_LANE_2 |
- LANE1_MAP_LOGIC_LANE_1 | LANE0_MAP_LOGIC_LANE_0;
+ LANE1_MAP_LOGIC_LANE_1 | LANE0_MAP_LOGIC_LANE_0;
writel(reg, dp->reg_base + ANALOGIX_DP_LANE_MAP);
}
@@ -202,8 +202,8 @@ void analogix_dp_set_pll_power_down(struct analogix_dp_device *dp, bool enable)
}
void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
- enum analog_power_block block,
- bool enable)
+ enum analog_power_block block,
+ bool enable)
{
u32 reg;
@@ -399,8 +399,8 @@ void analogix_dp_init_aux(struct analogix_dp_device *dp)
analogix_dp_reset_aux(dp);
/* Disable AUX transaction H/W retry */
- reg = AUX_BIT_PERIOD_EXPECTED_DELAY(3) | AUX_HW_RETRY_COUNT_SEL(0)|
- AUX_HW_RETRY_INTERVAL_600_MICROSECONDS;
+ reg = AUX_BIT_PERIOD_EXPECTED_DELAY(3) | AUX_HW_RETRY_COUNT_SEL(0) |
+ AUX_HW_RETRY_INTERVAL_600_MICROSECONDS;
writel(reg, dp->reg_base + ANALOGIX_DP_AUX_HW_RETRY_CTL);
/* Receive AUX Channel DEFER commands equal to DEFFER_COUNT*64 */
@@ -483,8 +483,8 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp)
}
int analogix_dp_write_byte_to_dpcd(struct analogix_dp_device *dp,
- unsigned int reg_addr,
- unsigned char data)
+ unsigned int reg_addr,
+ unsigned char data)
{
u32 reg;
int i;
@@ -519,17 +519,16 @@ int analogix_dp_write_byte_to_dpcd(struct analogix_dp_device *dp,
retval = analogix_dp_start_aux_transaction(dp);
if (retval == 0)
break;
- else
- dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
- __func__);
+
+ dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__);
}
return retval;
}
int analogix_dp_read_byte_from_dpcd(struct analogix_dp_device *dp,
- unsigned int reg_addr,
- unsigned char *data)
+ unsigned int reg_addr,
+ unsigned char *data)
{
u32 reg;
int i;
@@ -560,9 +559,8 @@ int analogix_dp_read_byte_from_dpcd(struct analogix_dp_device *dp,
retval = analogix_dp_start_aux_transaction(dp);
if (retval == 0)
break;
- else
- dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
- __func__);
+
+ dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__);
}
/* Read data buffer */
@@ -573,9 +571,9 @@ int analogix_dp_read_byte_from_dpcd(struct analogix_dp_device *dp,
}
int analogix_dp_write_bytes_to_dpcd(struct analogix_dp_device *dp,
- unsigned int reg_addr,
- unsigned int count,
- unsigned char data[])
+ unsigned int reg_addr,
+ unsigned int count,
+ unsigned char data[])
{
u32 reg;
unsigned int start_offset;
@@ -608,8 +606,9 @@ int analogix_dp_write_bytes_to_dpcd(struct analogix_dp_device *dp,
for (cur_data_idx = 0; cur_data_idx < cur_data_count;
cur_data_idx++) {
reg = data[start_offset + cur_data_idx];
- writel(reg, dp->reg_base + ANALOGIX_DP_BUF_DATA_0
- + 4 * cur_data_idx);
+ writel(reg, dp->reg_base +
+ ANALOGIX_DP_BUF_DATA_0 +
+ 4 * cur_data_idx);
}
/*
@@ -625,9 +624,9 @@ int analogix_dp_write_bytes_to_dpcd(struct analogix_dp_device *dp,
retval = analogix_dp_start_aux_transaction(dp);
if (retval == 0)
break;
- else
- dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
- __func__);
+
+ dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
+ __func__);
}
start_offset += cur_data_count;
@@ -637,9 +636,9 @@ int analogix_dp_write_bytes_to_dpcd(struct analogix_dp_device *dp,
}
int analogix_dp_read_bytes_from_dpcd(struct analogix_dp_device *dp,
- unsigned int reg_addr,
- unsigned int count,
- unsigned char data[])
+ unsigned int reg_addr,
+ unsigned int count,
+ unsigned char data[])
{
u32 reg;
unsigned int start_offset;
@@ -683,9 +682,9 @@ int analogix_dp_read_bytes_from_dpcd(struct analogix_dp_device *dp,
retval = analogix_dp_start_aux_transaction(dp);
if (retval == 0)
break;
- else
- dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
- __func__);
+
+ dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
+ __func__);
}
for (cur_data_idx = 0; cur_data_idx < cur_data_count;
@@ -703,8 +702,8 @@ int analogix_dp_read_bytes_from_dpcd(struct analogix_dp_device *dp,
}
int analogix_dp_select_i2c_device(struct analogix_dp_device *dp,
- unsigned int device_addr,
- unsigned int reg_addr)
+ unsigned int device_addr,
+ unsigned int reg_addr)
{
u32 reg;
int retval;
@@ -736,9 +735,9 @@ int analogix_dp_select_i2c_device(struct analogix_dp_device *dp,
}
int analogix_dp_read_byte_from_i2c(struct analogix_dp_device *dp,
- unsigned int device_addr,
- unsigned int reg_addr,
- unsigned int *data)
+ unsigned int device_addr,
+ unsigned int reg_addr,
+ unsigned int *data)
{
u32 reg;
int i;
@@ -750,7 +749,8 @@ int analogix_dp_read_byte_from_i2c(struct analogix_dp_device *dp,
writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL);
/* Select EDID device */
- retval = analogix_dp_select_i2c_device(dp, device_addr, reg_addr);
+ retval = analogix_dp_select_i2c_device(dp, device_addr,
+ reg_addr);
if (retval != 0)
continue;
@@ -767,9 +767,8 @@ int analogix_dp_read_byte_from_i2c(struct analogix_dp_device *dp,
retval = analogix_dp_start_aux_transaction(dp);
if (retval == 0)
break;
- else
- dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
- __func__);
+
+ dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__);
}
/* Read data */
@@ -780,10 +779,10 @@ int analogix_dp_read_byte_from_i2c(struct analogix_dp_device *dp,
}
int analogix_dp_read_bytes_from_i2c(struct analogix_dp_device *dp,
- unsigned int device_addr,
- unsigned int reg_addr,
- unsigned int count,
- unsigned char edid[])
+ unsigned int device_addr,
+ unsigned int reg_addr,
+ unsigned int count,
+ unsigned char edid[])
{
u32 reg;
unsigned int i, j;
@@ -828,15 +827,14 @@ int analogix_dp_read_bytes_from_i2c(struct analogix_dp_device *dp,
retval = analogix_dp_start_aux_transaction(dp);
if (retval == 0)
break;
- else
- dev_dbg(dp->dev,
- "%s: Aux Transaction fail!\n",
- __func__);
+
+ dev_dbg(dp->dev, "%s: Aux Transaction fail!\n",
+ __func__);
}
/* Check if Rx sends defer */
reg = readl(dp->reg_base + ANALOGIX_DP_AUX_RX_COMM);
if (reg == AUX_RX_COMM_AUX_DEFER ||
- reg == AUX_RX_COMM_I2C_DEFER) {
+ reg == AUX_RX_COMM_I2C_DEFER) {
dev_err(dp->dev, "Defer: %d\n\n", reg);
defer = 1;
}
@@ -885,7 +883,8 @@ void analogix_dp_get_lane_count(struct analogix_dp_device *dp, u32 *count)
*count = reg;
}
-void analogix_dp_enable_enhanced_mode(struct analogix_dp_device *dp, bool enable)
+void analogix_dp_enable_enhanced_mode(struct analogix_dp_device *dp,
+ bool enable)
{
u32 reg;
@@ -901,7 +900,7 @@ void analogix_dp_enable_enhanced_mode(struct analogix_dp_device *dp, bool enable
}
void analogix_dp_set_training_pattern(struct analogix_dp_device *dp,
- enum pattern_set pattern)
+ enum pattern_set pattern)
{
u32 reg;
@@ -933,7 +932,8 @@ void analogix_dp_set_training_pattern(struct analogix_dp_device *dp,
}
}
-void analogix_dp_set_lane0_pre_emphasis(struct analogix_dp_device *dp, u32 level)
+void analogix_dp_set_lane0_pre_emphasis(struct analogix_dp_device *dp,
+ u32 level)
{
u32 reg;
@@ -943,7 +943,8 @@ void analogix_dp_set_lane0_pre_emphasis(struct analogix_dp_device *dp, u32 level
writel(reg, dp->reg_base + ANALOGIX_DP_LN0_LINK_TRAINING_CTL);
}
-void analogix_dp_set_lane1_pre_emphasis(struct analogix_dp_device *dp, u32 level)
+void analogix_dp_set_lane1_pre_emphasis(struct analogix_dp_device *dp,
+ u32 level)
{
u32 reg;
@@ -953,7 +954,8 @@ void analogix_dp_set_lane1_pre_emphasis(struct analogix_dp_device *dp, u32 level
writel(reg, dp->reg_base + ANALOGIX_DP_LN1_LINK_TRAINING_CTL);
}
-void analogix_dp_set_lane2_pre_emphasis(struct analogix_dp_device *dp, u32 level)
+void analogix_dp_set_lane2_pre_emphasis(struct analogix_dp_device *dp,
+ u32 level)
{
u32 reg;
@@ -963,7 +965,8 @@ void analogix_dp_set_lane2_pre_emphasis(struct analogix_dp_device *dp, u32 level
writel(reg, dp->reg_base + ANALOGIX_DP_LN2_LINK_TRAINING_CTL);
}
-void analogix_dp_set_lane3_pre_emphasis(struct analogix_dp_device *dp, u32 level)
+void analogix_dp_set_lane3_pre_emphasis(struct analogix_dp_device *dp,
+ u32 level)
{
u32 reg;
@@ -974,7 +977,7 @@ void analogix_dp_set_lane3_pre_emphasis(struct analogix_dp_device *dp, u32 level
}
void analogix_dp_set_lane0_link_training(struct analogix_dp_device *dp,
- u32 training_lane)
+ u32 training_lane)
{
u32 reg;
@@ -983,7 +986,7 @@ void analogix_dp_set_lane0_link_training(struct analogix_dp_device *dp,
}
void analogix_dp_set_lane1_link_training(struct analogix_dp_device *dp,
- u32 training_lane)
+ u32 training_lane)
{
u32 reg;
@@ -1001,7 +1004,7 @@ void analogix_dp_set_lane2_link_training(struct analogix_dp_device *dp,
}
void analogix_dp_set_lane3_link_training(struct analogix_dp_device *dp,
- u32 training_lane)
+ u32 training_lane)
{
u32 reg;
@@ -1125,9 +1128,8 @@ int analogix_dp_is_slave_video_stream_clock_on(struct analogix_dp_device *dp)
}
void analogix_dp_set_video_cr_mn(struct analogix_dp_device *dp,
- enum clock_recovery_m_value_type type,
- u32 m_value,
- u32 n_value)
+ enum clock_recovery_m_value_type type,
+ u32 m_value, u32 n_value)
{
u32 reg;
@@ -1221,7 +1223,7 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp)
u32 reg;
reg = readl(dp->reg_base + ANALOGIX_DP_FUNC_EN_1);
- reg &= ~(MASTER_VID_FUNC_EN_N|SLAVE_VID_FUNC_EN_N);
+ reg &= ~(MASTER_VID_FUNC_EN_N | SLAVE_VID_FUNC_EN_N);
reg |= MASTER_VID_FUNC_EN_N;
writel(reg, dp->reg_base + ANALOGIX_DP_FUNC_EN_1);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2015-09-22 09:50 +0200 |
| Subject | [PATCH v5 06/17] Documentation: drm/bridge: add document for analogix_dp |
| Message-ID | <qbquR-7mj-1@gated-at.bofh.it> |
| In reply to | #1229939 |
Analogix dp driver is split from exynos dp driver, so we just
make an copy of exynos_dp.txt, and then simplify exynos_dp.txt
Beside update some exynos dtsi file with the latest change
according to the devicetree binding documents.
Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v5: None
Changes in v4:
- Split all DTS changes, and provide backward compatibility. Mark old
properties as deprecated but still support them. (Krzysztof)
- Update "analogix,hpd-gpio" to "hpd-gpios" prop name. (Rob)
- Deprecated some properties which could parsed from Edid/Mode/DPCD. (Thierry)
"analogix,color-space" & "analogix,color-depth" &
"analogix,link-rate" & "analogix,lane-count" &
"analogix,ycbcr-coeff" & "analogix,dynamic-range" &
"vsync-active-high" & "hsync-active-high" & "interlaces"
Changes in v3:
- Add devicetree binding documents. (Heiko)
- Remove sync pol & colorimetry properies from the new analogix dp driver
devicetree binding. (Thierry)
- Update the exist exynos dtsi file with the latest DP DT properies.
Changes in v2: None
.../devicetree/bindings/drm/bridge/analogix_dp.txt | 50 +++++++++++++++++
.../devicetree/bindings/video/exynos_dp.txt | 63 ++++++++--------------
2 files changed, 71 insertions(+), 42 deletions(-)
create mode 100644 Documentation/devicetree/bindings/drm/bridge/analogix_dp.txt
diff --git a/Documentation/devicetree/bindings/drm/bridge/analogix_dp.txt b/Documentation/devicetree/bindings/drm/bridge/analogix_dp.txt
new file mode 100644
index 0000000..f54dc3e
--- /dev/null
+++ b/Documentation/devicetree/bindings/drm/bridge/analogix_dp.txt
@@ -0,0 +1,50 @@
+Analogix Display Port bridge bindings
+
+Required properties for dp-controller:
+ -compatible:
+ platform specific such as:
+ * "samsung,exynos5-dp"
+ * "rockchip,rk3288-dp"
+ -reg:
+ physical base address of the controller and length
+ of memory mapped region.
+ -interrupts:
+ interrupt combiner values.
+ -clocks:
+ from common clock binding: handle to dp clock.
+ -clock-names:
+ from common clock binding: Shall be "dp".
+ -interrupt-parent:
+ phandle to Interrupt combiner node.
+ -phys:
+ from general PHY binding: the phandle for the PHY device.
+ -phy-names:
+ from general PHY binding: Should be "dp".
+
+Optional properties for dp-controller:
+ -hpd-gpios:
+ Hotplug detect GPIO.
+ Indicates which GPIO should be used for hotplug detection
+ -port@[X]: SoC specific port nodes with endpoint definitions as defined
+ in Documentation/devicetree/bindings/media/video-interfaces.txt,
+ please refer to the SoC specific binding document:
+ * Documentation/devicetree/bindings/video/exynos_dp.txt
+ * Documentation/devicetree/bindings/video/analogix_dp-rockchip.txt
+
+
+[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
+-------------------------------------------------------------------------------
+
+Example:
+
+ dp-controller {
+ compatible = "samsung,exynos5-dp";
+ reg = <0x145b0000 0x10000>;
+ interrupts = <10 3>;
+ interrupt-parent = <&combiner>;
+ clocks = <&clock 342>;
+ clock-names = "dp";
+
+ phys = <&dp_phy>;
+ phy-names = "dp";
+ };
diff --git a/Documentation/devicetree/bindings/video/exynos_dp.txt b/Documentation/devicetree/bindings/video/exynos_dp.txt
index 7a3a9cd..ea03b3a 100644
--- a/Documentation/devicetree/bindings/video/exynos_dp.txt
+++ b/Documentation/devicetree/bindings/video/exynos_dp.txt
@@ -31,45 +31,31 @@ Required properties for dp-controller:
from general PHY binding: the phandle for the PHY device.
-phy-names:
from general PHY binding: Should be "dp".
- -samsung,color-space:
- input video data format.
- COLOR_RGB = 0, COLOR_YCBCR422 = 1, COLOR_YCBCR444 = 2
- -samsung,dynamic-range:
- dynamic range for input video data.
- VESA = 0, CEA = 1
- -samsung,ycbcr-coeff:
- YCbCr co-efficients for input video.
- COLOR_YCBCR601 = 0, COLOR_YCBCR709 = 1
- -samsung,color-depth:
- number of bits per colour component.
- COLOR_6 = 0, COLOR_8 = 1, COLOR_10 = 2, COLOR_12 = 3
- -samsung,link-rate:
- link rate supported by the panel.
- LINK_RATE_1_62GBPS = 0x6, LINK_RATE_2_70GBPS = 0x0A
- -samsung,lane-count:
- number of lanes supported by the panel.
- LANE_COUNT1 = 1, LANE_COUNT2 = 2, LANE_COUNT4 = 4
+
+Optional properties for dp-controller:
- display-timings: timings for the connected panel as described by
Documentation/devicetree/bindings/video/display-timing.txt
-Optional properties for dp-controller:
- -interlaced:
- interlace scan mode.
- Progressive if defined, Interlaced if not defined
- -vsync-active-high:
- VSYNC polarity configuration.
- High if defined, Low if not defined
- -hsync-active-high:
- HSYNC polarity configuration.
- High if defined, Low if not defined
- -samsung,hpd-gpio:
- Hotplug detect GPIO.
- Indicates which GPIO should be used for hotplug
- detection
- -video interfaces: Device node can contain video interface port
- nodes according to [1].
+For the below properties, please refer to Analogix DP binding document:
+ * Documentation/devicetree/bindings/drm/bridge/analogix_dp.txt
+ -phys (required)
+ -phy-names (required)
+ -hpd-gpios (optional)
+ -video interfaces (optional)
-[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
+Deprecated properties for DisplayPort:
+-interlaced: deprecated prop that can parsed frm drm_display_mode.
+-vsync-active-high: deprecated prop that can parsed frm drm_display_mode.
+-hsync-active-high: deprecated prop that can parsed frm drm_display_mode.
+-samsung,ycbcr-coeff: deprecated prop that can parsed frm drm_display_mode.
+-samsung,dynamic-range: deprecated prop that can parsed frm drm_display_mode.
+-samsung,color-space: deprecated prop that can parsed from drm_display_info.
+-samsung,color-depth: deprecated prop that can parsed from drm_display_info.
+-samsung,link-rate: deprecated prop that can reading from monitor by dpcd method.
+-samsung,lane-count: deprecated prop that can reading from monitor by dpcd method.
+-samsung,hpd-gpio: deprecated name for hpd-gpios.
+
+-------------------------------------------------------------------------------
Example:
@@ -88,13 +74,6 @@ SOC specific portion:
Board Specific portion:
dp-controller {
- samsung,color-space = <0>;
- samsung,dynamic-range = <0>;
- samsung,ycbcr-coeff = <0>;
- samsung,color-depth = <1>;
- samsung,link-rate = <0x0a>;
- samsung,lane-count = <4>;
-
display-timings {
native-mode = <&lcd_timing>;
lcd_timing: 1366x768 {
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Yakir Yang <ykk@rock-chips.com> |
|---|---|
| Date | 2015-09-22 09:50 +0200 |
| Subject | [PATCH v5 07/17] ARM: dts: exynos/dp: remove some properties that deprecated by analogix_dp driver |
| Message-ID | <qbquR-7mj-5@gated-at.bofh.it> |
| In reply to | #1229939 |
After exynos_dp have been split the common IP code into analogix_dp driver,
the analogix_dp driver have deprecated some Samsung platform properties which
could be dynamically parsed from EDID/MODE/DPCD message, so this is an update
for Exynos DTS file for dp-controller.
Beside the backward compatibility is fully preserved, so there are no
bisectability break that make this change in a separate patch.
Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v5:
- Correct the misspell in commit message. (Krzysztof)
Changes in v4:
- Separate all DTS changes to a separate patch. (Krzysztof)
Changes in v3: None
Changes in v2: None
arch/arm/boot/dts/exynos5250-arndale.dts | 2 --
arch/arm/boot/dts/exynos5250-smdk5250.dts | 2 --
arch/arm/boot/dts/exynos5250-snow.dts | 4 +---
arch/arm/boot/dts/exynos5250-spring.dts | 4 +---
arch/arm/boot/dts/exynos5420-peach-pit.dts | 4 +---
arch/arm/boot/dts/exynos5420-smdk5420.dts | 2 --
arch/arm/boot/dts/exynos5800-peach-pi.dts | 4 +---
7 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts
index db3f65f..4636862 100644
--- a/arch/arm/boot/dts/exynos5250-arndale.dts
+++ b/arch/arm/boot/dts/exynos5250-arndale.dts
@@ -124,8 +124,6 @@
&dp {
status = "okay";
samsung,color-space = <0>;
- samsung,dynamic-range = <0>;
- samsung,ycbcr-coeff = <0>;
samsung,color-depth = <1>;
samsung,link-rate = <0x0a>;
samsung,lane-count = <4>;
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index c625e71..cd424d6 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -80,8 +80,6 @@
&dp {
samsung,color-space = <0>;
- samsung,dynamic-range = <0>;
- samsung,ycbcr-coeff = <0>;
samsung,color-depth = <1>;
samsung,link-rate = <0x0a>;
samsung,lane-count = <4>;
diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
index 0720caa..242b621 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -244,12 +244,10 @@
pinctrl-names = "default";
pinctrl-0 = <&dp_hpd>;
samsung,color-space = <0>;
- samsung,dynamic-range = <0>;
- samsung,ycbcr-coeff = <0>;
samsung,color-depth = <1>;
samsung,link-rate = <0x0a>;
samsung,lane-count = <2>;
- samsung,hpd-gpio = <&gpx0 7 GPIO_ACTIVE_HIGH>;
+ hpd-gpios = <&gpx0 7 GPIO_ACTIVE_HIGH>;
ports {
port@0 {
diff --git a/arch/arm/boot/dts/exynos5250-spring.dts b/arch/arm/boot/dts/exynos5250-spring.dts
index c1edd6d..91881d7 100644
--- a/arch/arm/boot/dts/exynos5250-spring.dts
+++ b/arch/arm/boot/dts/exynos5250-spring.dts
@@ -74,12 +74,10 @@
pinctrl-names = "default";
pinctrl-0 = <&dp_hpd_gpio>;
samsung,color-space = <0>;
- samsung,dynamic-range = <0>;
- samsung,ycbcr-coeff = <0>;
samsung,color-depth = <1>;
samsung,link-rate = <0x0a>;
samsung,lane-count = <1>;
- samsung,hpd-gpio = <&gpc3 0 GPIO_ACTIVE_HIGH>;
+ hpd-gpios = <&gpc3 0 GPIO_ACTIVE_HIGH>;
};
&ehci {
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 8f4d76c..7433683 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -148,12 +148,10 @@
pinctrl-names = "default";
pinctrl-0 = <&dp_hpd_gpio>;
samsung,color-space = <0>;
- samsung,dynamic-range = <0>;
- samsung,ycbcr-coeff = <0>;
samsung,color-depth = <1>;
samsung,link-rate = <0x06>;
samsung,lane-count = <2>;
- samsung,hpd-gpio = <&gpx2 6 0>;
+ hpd-gpio = <&gpx2 6 0>;
ports {
port@0 {
diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts b/arch/arm/boot/dts/exynos5420-smdk5420.dts
index 98871f9..b3df3c1 100644
--- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
+++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
@@ -92,8 +92,6 @@
pinctrl-names = "default";
pinctrl-0 = <&dp_hpd>;
samsung,color-space = <0>;
- samsung,dynamic-range = <0>;
- samsung,ycbcr-coeff = <0>;
samsung,color-depth = <1>;
samsung,link-rate = <0x0a>;
samsung,lane-count = <4>;
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index 7d5b386..b89bff5 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -142,12 +142,10 @@
pinctrl-names = "default";
pinctrl-0 = <&dp_hpd_gpio>;
samsung,color-space = <0>;
- samsung,dynamic-range = <0>;
- samsung,ycbcr-coeff = <0>;
samsung,color-depth = <1>;
samsung,link-rate = <0x0a>;
samsung,lane-count = <2>;
- samsung,hpd-gpio = <&gpx2 6 0>;
+ hpd-gpios = <&gpx2 6 0>;
panel = <&panel>;
};
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
Page 4 of 5 — ← Prev page 1 2 3 [4] 5 Next page →
Back to top | Article view | linux.kernel
csiph-web