Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1367606 > unrolled thread

Re: [v14, 01/17] drm: bridge: analogix/dp: split exynos dp driver to bridge directory

Started byGuenter Roeck <linux@roeck-us.net>
First post2016-03-30 22:40 +0200
Last post2016-03-31 18:00 +0200
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [v14, 01/17] drm: bridge: analogix/dp: split exynos dp driver to  bridge directory Guenter Roeck <linux@roeck-us.net> - 2016-03-30 22:40 +0200
    Re: [v14, 01/17] drm: bridge: analogix/dp: split exynos dp driver to  bridge directory Thierry Reding <treding@nvidia.com> - 2016-03-31 12:00 +0200
      Re: [v14, 01/17] drm: bridge: analogix/dp: split exynos dp driver to  bridge directory Doug Anderson <dianders@chromium.org> - 2016-03-31 18:10 +0200
    Re: [v14, 01/17] drm: bridge: analogix/dp: split exynos dp driver to  bridge directory Doug Anderson <dianders@chromium.org> - 2016-03-31 18:00 +0200

#1367606 — Re: [v14, 01/17] drm: bridge: analogix/dp: split exynos dp driver to bridge directory

FromGuenter Roeck <linux@roeck-us.net>
Date2016-03-30 22:40 +0200
SubjectRe: [v14, 01/17] drm: bridge: analogix/dp: split exynos dp driver to bridge directory
Message-ID<riv4e-2lN-15@gated-at.bofh.it>
Hi,

On Mon, Feb 15, 2016 at 07:09:36PM +0800, Yakir Yang wrote:
> Split the dp core driver from exynos directory to bridge directory,
> and rename the core driver to analogix_dp_*, rename the platform
> code to exynos_dp.
> 
> Beside the new analogix_dp driver would export six hooks.
> "analogix_dp_bind()" and "analogix_dp_unbind()"
> "analogix_dp_suspned()" and "analogix_dp_resume()"
> "analogix_dp_detect()" and "analogix_dp_get_modes()"
> 
> The bind/unbind symbols is used for analogix platform driver to connect
> with analogix_dp core driver. And the detect/get_modes is used for analogix
> platform driver to init the connector.
> 
> They reason why connector need register in helper driver is rockchip drm
> haven't implement the atomic API, but Exynos drm have implement it, so
> there would need two different connector helper functions, that's why we
> leave the connector register in helper driver.
> 
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> 
[ ... ]

> 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

This results in the following build errors if DRM_ANALOGIX_DP
is configured as module.

ERROR: "analogix_dp_start_video" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
ERROR: "analogix_dp_get_lane0_link_training" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
ERROR: "analogix_dp_get_lane1_link_training" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
ERROR: "analogix_dp_get_lane2_link_training" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
ERROR: "analogix_dp_get_lane3_link_training" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
ERROR: "analogix_dp_get_lane_count" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
ERROR: "analogix_dp_get_link_bandwidth" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!

[ and so on ]

It should probably be something like

analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o
obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o

Guenter

[toc] | [next] | [standalone]


#1368127

FromThierry Reding <treding@nvidia.com>
Date2016-03-31 12:00 +0200
Message-ID<riHyq-32C-21@gated-at.bofh.it>
In reply to#1367606

[Multipart message — attachments visible in raw view] — view raw

On Wed, Mar 30, 2016 at 01:32:04PM -0700, Guenter Roeck wrote:
> Hi,
> 
> On Mon, Feb 15, 2016 at 07:09:36PM +0800, Yakir Yang wrote:
> > Split the dp core driver from exynos directory to bridge directory,
> > and rename the core driver to analogix_dp_*, rename the platform
> > code to exynos_dp.
> > 
> > Beside the new analogix_dp driver would export six hooks.
> > "analogix_dp_bind()" and "analogix_dp_unbind()"
> > "analogix_dp_suspned()" and "analogix_dp_resume()"
> > "analogix_dp_detect()" and "analogix_dp_get_modes()"
> > 
> > The bind/unbind symbols is used for analogix platform driver to connect
> > with analogix_dp core driver. And the detect/get_modes is used for analogix
> > platform driver to init the connector.
> > 
> > They reason why connector need register in helper driver is rockchip drm
> > haven't implement the atomic API, but Exynos drm have implement it, so
> > there would need two different connector helper functions, that's why we
> > leave the connector register in helper driver.
> > 
> > Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> > 
> [ ... ]
> 
> > 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
> 
> This results in the following build errors if DRM_ANALOGIX_DP
> is configured as module.
> 
> ERROR: "analogix_dp_start_video" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
> ERROR: "analogix_dp_get_lane0_link_training" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
> ERROR: "analogix_dp_get_lane1_link_training" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
> ERROR: "analogix_dp_get_lane2_link_training" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
> ERROR: "analogix_dp_get_lane3_link_training" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
> ERROR: "analogix_dp_get_lane_count" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
> ERROR: "analogix_dp_get_link_bandwidth" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
> 
> [ and so on ]

Ugh... most of these functions shouldn't be there in the first place. We
have helpers in the core that already do this. Most of the functionality
is duplicated in this driver.

I realize that this is a problem that existed in the Exynos DP driver,
but somebody really ought to rewrite those parts to make use of the DRM
DP helpers.

Thierry

[toc] | [prev] | [next] | [standalone]


#1368419

FromDoug Anderson <dianders@chromium.org>
Date2016-03-31 18:10 +0200
Message-ID<riNku-7EN-5@gated-at.bofh.it>
In reply to#1368127
Hi,

On Thu, Mar 31, 2016 at 2:56 AM, Thierry Reding <treding@nvidia.com> wrote:
> Ugh... most of these functions shouldn't be there in the first place. We
> have helpers in the core that already do this. Most of the functionality
> is duplicated in this driver.
>
> I realize that this is a problem that existed in the Exynos DP driver,
> but somebody really ought to rewrite those parts to make use of the DRM
> DP helpers.

As I understand it from discussions in
<http://article.gmane.org/gmane.linux.drivers.devicetree/162059> and
<http://article.gmane.org/gmane.linux.kernel.samsung-soc/53048>, we
shouldn't block merging the series but someone should pick up this
work after the merge.

So probably Yakir should fixup the problem pointed out by Guenter and
send a new pull.

-Doug

[toc] | [prev] | [next] | [standalone]


#1368412

FromDoug Anderson <dianders@chromium.org>
Date2016-03-31 18:00 +0200
Message-ID<riNaN-7j4-1@gated-at.bofh.it>
In reply to#1367606
Hi,

On Wed, Mar 30, 2016 at 1:32 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> This results in the following build errors if DRM_ANALOGIX_DP
> is configured as module.
>
> ERROR: "analogix_dp_start_video" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
> ERROR: "analogix_dp_get_lane0_link_training" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
> ERROR: "analogix_dp_get_lane1_link_training" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
> ERROR: "analogix_dp_get_lane2_link_training" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
> ERROR: "analogix_dp_get_lane3_link_training" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
> ERROR: "analogix_dp_get_lane_count" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
> ERROR: "analogix_dp_get_link_bandwidth" [drivers/gpu/drm/bridge/analogix/analogix_dp_core.ko] undefined!
>
> [ and so on ]
>
> It should probably be something like
>
> analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o
> obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o

Sounds like this is a major problem.  Presumably Yakir should fix and
send a new pull request?

-Doug

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web