Re: [linux-sunxi] [PATCH 1/6] drm/bridge: Export drm_bridge_detach

[Jagan Teki <jagan@amarulasolutions.com>]

On Mon, Mar 18, 2019 at 10:27 PM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> Hi,
>
> Le lundi 18 mars 2019 à 22:18 +0530, Jagan Teki a écrit :
> > Hi Paul,
> >
> > On Fri, Mar 15, 2019 at 6:58 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > > Hi Jakan,
> > >
> > > On Fri, 2019-03-15 at 18:38 +0530, Jagan Teki wrote:
> > > > Export drm_bridge_detach from drm bridge core so-that it
> > > > can use on respective interface or bridge driver while
> > > > detaching the bridge.
> > >
> > > I don't see why this change is required based on the commit log. The
> > > DRM bridge code clearly indicates that drm_bridge_attach should *not*
> > > be balanced with a drm_bridge_detach call in the driver, so this seems
> > > quite wrong.
> > >
> > > The DRM core itself should handle detaching the bridge, not the driver.
> > > Is there any reason why you need to do things differently for DSI?
> >
> > Yes, you are correct the detach of bridge is being taking care via
> > drm_encoder_cleanup. This patch exported explicitly, since we need to
> > taken care bridge detach during unbind even exynos_drm_dsi in other
> > patch seems using detach by explicitly pointing.
>
> I can see that from your patches, but you are not explaining why you
> need the change. And if the framework doesn't work for your case, you
> should certainly try and fix the framework instead of working around
> the issue.

Please see below comments.

>
> Anyway, you should probably look into using drm_panel_bridge_add, it
> might fix the underlying issue on its own.
>
> > so I think the better approach is to use drm_encoder_cleanup in
> > unbind, what do you say?
>
> I any case, you need to state what your problem is (in the commit log,
> and not even in subsequent discussions) so that we can have a chance to
> understand it and provide some feedback about what is an appropriate
> fix and what is not. We can't understand the fix if we can't understand
> the underlying issue.

True, if my intentions is trying to fix some issue, but as I said in
previous mail since the other dsi (exynos_drm_dsi) driver is
explicitly detaching I presume it require or missed the export, not
thought about fix and neither I mentioned on the commit head.  Anyway
thanks for the review, will update the code accordingly in next
version.