[v2,12/26] drm/exynos: Split manager/display/subdrv

Message ID	1381951616-12548-13-git-send-email-seanpaul@chromium.org (mailing list archive)
State	New, archived
Headers	show Return-Path: <dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org> From: Sean Paul <seanpaul@chromium.org> To: dri-devel@lists.freedesktop.org, inki.dae@samsung.com Subject: [PATCH v2 12/26] drm/exynos: Split manager/display/subdrv Date: Wed, 16 Oct 2013 15:26:42 -0400 Message-Id: <1381951616-12548-13-git-send-email-seanpaul@chromium.org> In-Reply-To: <1381951616-12548-1-git-send-email-seanpaul@chromium.org> References: <1381951616-12548-1-git-send-email-seanpaul@chromium.org> Cc: marcheu@chromium.org Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org

2013/10/23 Sean Paul <seanpaul@chromium.org>: > On Wed, Oct 23, 2013 at 1:42 AM, Inki Dae <inki.dae@samsung.com> wrote: >> 2013/10/23 Sean Paul <seanpaul@chromium.org>: >>> On Wed, Oct 23, 2013 at 12:48 AM, Inki Dae <inki.dae@samsung.com> wrote: >>>> 2013/10/23 Stéphane Marchesin <stephane.marchesin@gmail.com>: >>>>> >>>>> >>>>> >>>>> On Tue, Oct 22, 2013 at 9:15 PM, Inki Dae <inki.dae@samsung.com> wrote: >>>>>> >>>>>> 2013/10/23 Stéphane Marchesin <stephane.marchesin@gmail.com>: >>>>>> > >>>>>> > >>>>>> > >>>>>> > On Tue, Oct 22, 2013 at 8:38 PM, Inki Dae <inki.dae@samsung.com> wrote: >>>>>> >> >>>>>> >> 2013/10/23 Stéphane Marchesin <stephane.marchesin@gmail.com>: >>>>>> >> > >>>>>> >> > >>>>>> >> > >>>>>> >> > On Tue, Oct 22, 2013 at 7:28 PM, Inki Dae <inki.dae@samsung.com> >>>>>> >> > wrote: >>>>>> >> >> >>>>>> >> >> 2013/10/22 Sean Paul <seanpaul@chromium.org>: >>>>>> >> >> > On Tue, Oct 22, 2013 at 1:30 AM, Inki Dae <inki.dae@samsung.com> >>>>>> >> >> > wrote: >>>>>> >> >> >> >>>>>> >> >> >> >>>>>> >> >> >>> -----Original Message----- >>>>>> >> >> >>> From: Sean Paul [mailto:seanpaul@chromium.org] >>>>>> >> >> >>> Sent: Tuesday, October 22, 2013 6:18 AM >>>>>> >> >> >>> To: Inki Dae >>>>>> >> >> >>> Cc: dri-devel; Dave Airlie; Tomasz Figa; Stéphane Marchesin >>>>>> >> >> >>> Subject: Re: [PATCH v2 12/26] drm/exynos: Split >>>>>> >> >> >>> manager/display/subdrv >>>>>> >> >> >>> >>>>>> >> >> >>> On Mon, Oct 21, 2013 at 10:46 AM, Sean Paul >>>>>> >> >> >>> <seanpaul@chromium.org> >>>>>> >> >> >>> wrote: >>>>>> >> >> >>> > On Thu, Oct 17, 2013 at 10:31 PM, Inki Dae >>>>>> >> >> >>> > <inki.dae@samsung.com> >>>>>> >> >> >>> > wrote: >>>>>> >> >> >>> >> >>>>>> >> >> >>> >> >>>>>> >> >> >>> >>> -----Original Message----- >>>>>> >> >> >>> >>> From: Sean Paul [mailto:seanpaul@chromium.org] >>>>>> >> >> >>> >>> Sent: Thursday, October 17, 2013 11:37 PM >>>>>> >> >> >>> >>> To: Inki Dae >>>>>> >> >> >>> >>> Cc: dri-devel; Dave Airlie; Tomasz Figa; Stéphane Marchesin >>>>>> >> >> >>> >>> Subject: Re: [PATCH v2 12/26] drm/exynos: Split >>>>>> >> >> >>> >>> manager/display/subdrv >>>>>> >> >> >>> >>> >>>>>> >> >> >>> >>> On Thu, Oct 17, 2013 at 4:21 AM, Inki Dae >>>>>> >> >> >>> >>> <inki.dae@samsung.com> >>>>>> >> >> >> wrote: >>>>>> >> >> >>> >>> > >>>>>> >> >> >>> >>> > >>>>>> >> >> >>> >>> >> -----Original Message----- >>>>>> >> >> >>> >>> >> From: Sean Paul [mailto:seanpaul@chromium.org] >>>>>> >> >> >>> >>> >> Sent: Thursday, October 17, 2013 4:27 AM >>>>>> >> >> >>> >>> >> To: dri-devel@lists.freedesktop.org; inki.dae@samsung.com >>>>>> >> >> >>> >>> >> Cc: airlied@linux.ie; tomasz.figa@gmail.com; >>>>>> >> >> >>> >>> >> marcheu@chromium.org; >>>>>> >> >> >>> Sean >>>>>> >> >> >>> >>> >> Paul >>>>>> >> >> >>> >>> >> Subject: [PATCH v2 12/26] drm/exynos: Split >>>>>> >> >> >>> >>> >> manager/display/subdrv >>>>>> >> >> >>> >>> >> >>>>>> >> >> >>> >>> >> This patch splits display and manager from subdrv. The >>>>>> >> >> >>> >>> >> result >>>>>> >> >> >>> >>> >> is >>>>>> >> >> >>> that >>>>>> >> >> >>> >>> >> crtc functions can directly call into manager callbacks >>>>>> >> >> >>> >>> >> and >>>>>> >> >> >>> >>> >> encoder >>>>>> >> >> >>> >>> >> functions can directly call into display callbacks. This >>>>>> >> >> >>> >>> >> will >>>>>> >> >> >>> >>> >> allow >>>>>> >> >> >>> >>> >> us to remove the exynos_drm_hdmi shim and support >>>>>> >> >> >>> >>> >> mixer/hdmi >>>>>> >> >> >>> >>> >> & >>>>>> >> >> >>> fimd/dp >>>>>> >> >> >>> >>> >> with common code. >>>>>> >> >> >>> >>> >> >>>>>> >> >> >>> >>> >> Signed-off-by: Sean Paul <seanpaul@chromium.org> >>>>>> >> >> >>> >>> >> --- >>>>>> >> >> >>> >>> >> >>>>>> >> >> >>> >>> >> Changes in v2: >>>>>> >> >> >>> >>> >> - Pass display into display_ops instead of context >>>>>> >> >> >>> >>> > >>>>>> >> >> >>> >>> > Sorry but it seems like more reasonable to pass device >>>>>> >> >> >>> >>> > object >>>>>> >> >> >>> >>> > into >>>>>> >> >> >>> >>> > display_ops and manager_ops. >>>>>> >> >> >>> >>> > >>>>>> >> >> >>> >>> >>>>>> >> >> >>> >>> >>>>>> >> >> >>> >>> So you've changed your mind from when you said the >>>>>> >> >> >>> >>> following? >>>>>> >> >> >>> >>> >>>>>> >> >> >>> >>> >>> manager->ops->xxx(manager, ...); >>>>>> >> >> >>> >>> >>> display->ops->xxx(display, ...); >>>>>> >> >> >>> >>> >>> >>>>>> >> >> >>> >>> >>> Agree. >>>>>> >> >> >>> >>> >>>>>> >> >> >>> >> >>>>>> >> >> >>> >> >>>>>> >> >> >>> >> True. Before that, My comment was to pass device object into >>>>>> >> >> >>> display_ops and >>>>>> >> >> >>> >> manager_ops, and then you said the good solution is to pass >>>>>> >> >> >>> >> manager >>>>>> >> >> >>> >> and >>>>>> >> >> >>> >> display to each driver. At that time, I thought no matter how >>>>>> >> >> >>> >> the >>>>>> >> >> >>> callback >>>>>> >> >> >>> >> is called if the framework doesn't call callbacks of each >>>>>> >> >> >>> >> driver >>>>>> >> >> >>> >> with >>>>>> >> >> >>> ctx. >>>>>> >> >> >>> >> So I agreed. >>>>>> >> >> >>> >> >>>>>> >> >> >>> >> >>>>>> >> >> >>> >>> It would have been nice if you had changed your mind >>>>>> >> >> >>> >>> *before* I >>>>>> >> >> >>> >>> reworked everything. At any rate, I think it's still the >>>>>> >> >> >>> >>> right >>>>>> >> >> >>> >>> thing >>>>>> >> >> >>> >>> to do. >>>>>> >> >> >>> >> >>>>>> >> >> >>> >> Really sorry about that. And I will add new patch for it so >>>>>> >> >> >>> >> you >>>>>> >> >> >>> >> don't >>>>>> >> >> >>> need >>>>>> >> >> >>> >> to concern about that. >>>>>> >> >> >>> >> >>>>>> >> >> >>> >>> >>>>>> >> >> >>> >>> >>>>>> >> >> >>> >>> > I'm not sure but display_ops could be implemented in other >>>>>> >> >> >>> >>> > framework >>>>>> >> >> >>> >>> based >>>>>> >> >> >>> >>> > driver such as CDF based lcd panel driver. So if you pass >>>>>> >> >> >>> >>> > display - >>>>>> >> >> >>> it's >>>>>> >> >> >>> >>> > specific to exynos drm framework - into display_ops, the >>>>>> >> >> >>> >>> > other >>>>>> >> >> >>> framework >>>>>> >> >> >>> >>> > based driver should include specific exynos drm header. >>>>>> >> >> >>> >>> > >>>>>> >> >> >>> >>> >>>>>> >> >> >>> >>> AFAIK, CDF will not land in its current separate-from-drm >>>>>> >> >> >>> >>> form, >>>>>> >> >> >>> >>> we >>>>>> >> >> >>> >>> don't need to worry about this. Furthermore, these ops >>>>>> >> >> >>> >>> should >>>>>> >> >> >>> >>> just >>>>>> >> >> >>> >>> go >>>>>> >> >> >>> >>> away and become drm_crtc/drm_encoder/drm_connector funcs >>>>>> >> >> >>> >>> anyways. >>>>>> >> >> >>> >>> >>>>>> >> >> >>> >> >>>>>> >> >> >>> >> Can you assure the display_ops never implemented in other >>>>>> >> >> >>> >> framework >>>>>> >> >> >>> based >>>>>> >> >> >>> >> driver, not CDF? At any rate, I think all possibilities >>>>>> >> >> >>> >> should >>>>>> >> >> >>> >> be >>>>>> >> >> >>> opened. >>>>>> >> >> >>> >> >>>>>> >> >> >>> > >>>>>> >> >> >>> > I don't think we should let an RFC framework make the code >>>>>> >> >> >>> > more >>>>>> >> >> >>> > complicated for unclear benefit. By removing manager/display >>>>>> >> >> >>> > entirely, >>>>>> >> >> >>> > we can get rid of a *lot* of other code that is basically just >>>>>> >> >> >>> > plumbing drm hooks (exynos_drm_connector is a good example). >>>>>> >> >> >>> > >>>>>> >> >> >>> >>>>>> >> >> >>> I hacked this up today to prove it out. Check out the top 5 >>>>>> >> >> >>> commits >>>>>> >> >> >>> in >>>>>> >> >> >>> >>>>>> >> >> >>> >>>>>> >> >> >>> >>>>>> >> >> >>> https://github.com/crseanpaul/exynos-drm-next/commits/linux-next-exynos- >>>>>> >> >> >>> staging. >>>>>> >> >> >>> It removes exynos_drm_connector in favor of just implementing >>>>>> >> >> >>> drm_connector directly. This same treatment should be done for >>>>>> >> >> >>> exynos_drm_encoder and exynos_drm_crtc, but I didn't get around >>>>>> >> >> >>> to >>>>>> >> >> >>> doing this. >>>>>> >> >> >>> >>>>>> >> >> >>> As you can see, it cuts out a lot of code and removes an entire >>>>>> >> >> >>> abstraction layer. Much nicer :) >>>>>> >> >> >>> >>>>>> >> >> >> >>>>>> >> >> >> It seems that you implements connector in each device driver. >>>>>> >> >> >> Can't >>>>>> >> >> >> they be >>>>>> >> >> >> combined as common spot, exynos_connector, again to avoid codes >>>>>> >> >> >> from >>>>>> >> >> >> duplicated? :) >>>>>> >> >> > >>>>>> >> >> > There's nothing of substance being duplicated. >>>>>> >> >> >>>>>> >> >> Not true. xxx_create_connector is duplicated. >>>>>> >> >> >>>>>> >> >> > In fact, by getting rid >>>>>> >> >> > of the exynos_drm_connector layer, we deleted 150 lines. If you >>>>>> >> >> > really >>>>>> >> >> > take a look at exynos_drm_connector, it's not doing anything >>>>>> >> >> > useful. >>>>>> >> >> >>>>>> >> >> No, That is for each driver has no any dependency of drm framework. >>>>>> >> >> >>>>>> >> >> > All it does is translate the drm callbacks into display callbacks, >>>>>> >> >> > so >>>>>> >> >> > I think it's much better to just implement the drm callbacks >>>>>> >> >> > directly. >>>>>> >> >> > >>>>>> >> >> >>>>>> >> >> No, It has strongly dependency of drm framework. Assume that we >>>>>> >> >> implemented the drm callbacks directly, and then some features are >>>>>> >> >> added to drm framework, drm_connector side. At this time, we will >>>>>> >> >> have >>>>>> >> >> to take care of each device driver according to the change. That is >>>>>> >> >> really not good. Why device drivers should have dependency of drm >>>>>> >> >> framework? Just to reduce line counts? >>>>>> >> > >>>>>> >> > >>>>>> >> > >>>>>> >> > You seem to miss the point here and elsewhere in the discussion. >>>>>> >> > drm/exynos is a drm driver, and as such it should use the drm >>>>>> >> > framework, >>>>>> >> >>>>>> >> Hm.. you seem to miss something. Exynos drm based drivers are based on >>>>>> >> exynos drm framework, not drm framework directly. So I mean that >>>>>> >> Exynos drm framework based drivers should include only Exynos drm >>>>>> >> headers, _not drm header_ directly. >>>>>> > >>>>>> > >>>>>> > Well, I think everyone sees that exynos is different. But my point still >>>>>> > remains: why is the exynos driver in drm/ if it wants to use a different >>>>>> > framework? Right now it is blocking work on a proper drm driver... >>>>>> > >>>>>> >>>>>> Noooooo. It's not to use a different framework. It's to use a wrapper >>>>>> instead. >>>>> >>>>> >>>>> Ok, if you want to call it a wrapper, then what is the point of doing this >>>>> wrapping given that it prevents a proper drm-style implementation? >>>>> >>>> >>>> I already commented. That is for only Exynos drm framework has >>>> dependency of drm framework directly, and Exynos drm based drivers >>>> include only Exynos drm headers. >>>> >>>>> >>>>>> >>>>>> > >>>>>> >> >>>>>> >> >>>>>> >> > especially if this reduces the line count and the code >>>>>> >> > complexity (as is the case for this patch series). If you don't want >>>>>> >> > to maintain a drm driver, it simply should be moved away from drm/, >>>>>> >> > and it should be replaced by a real drm driver in my opinion. >>>>>> >> >>>>>> >> So those drivers should be in drm/exynos. Isn't that you really mean >>>>>> >> those drivers should be driver/gpu/drm? >>>>>> > >>>>>> > >>>>>> > I don't understand this sentence, sorry. >>>>>> >>>>>> Sorry, again, you mean Exynos drm based drivers should be in >>>>>> drivers/gpu/drm, not drivers/gpu/drm/exynos? >>>>>> >>>>> Is the exynos drm useful in its current shape at all? My recommendation >>>>> would be to fork off a real drm driver in gpu/drm/exynos with the current >>>>> code as a base. >>>>> >>>> >>>> Yes as of now. of course, There could be a better way. However, I >>>> don't want for Exynos drm based drivers have dependency of drm >>>> framework directly. >>>> >>> >>> Just to satisfy my curiosity, do you actually have something that uses >>> these drivers outside of drm? >>> >> >> Never no. >> >> >>> So I think we've reached somewhat of an impasse. I'd like to move the >>> driver towards a proper drm driver (ie: no exynos >>> framework/wrapper/whatever), you'd like to keep things separate. So >>> should we create a new exynos driver drivers/drm/gpu/exynos5 to house >>> the drm driver? >> >> Did you check out other drm drivers using similar way? And If so, the >> other drm framework based drivers should be moved? > > No, I haven't looked at other ARM drivers. It's besides the point, Look at omapdrm, nouveau, and radeon drm drivers. > really, that's not a compelling argument. For the exynos driver, in > particular, it doesn't make sense to wrapping everything in > exynos_drm_* That is really not thing to make sense or not. If you think this wrapping doesn't make sense, then can you argue about that the above drivers; omapdrm, nouveau, and radeon don't also make sense? That would definitely be just their style. If this design has some problem so the above drm maintainers also want to change their design, then I will merge your patch set. > >> What is the problem >> that Exynos drm based drivers include only Exynos drm headers, not drm >> framework header directly? Is that really a big issue? I _cannot_ >> understand your behavior. > > The problem is that it makes things unnecessarily complex for no benefit. > > For example, that you wanted to implement > connector_funcs->set_property() just in the hdmi driver. This requires > that you add a new display_op in exynos_drm_drv.h, then you implement > the connector_func callback in exynos_drm_connector to check if the op > is implemented then call it, and then you implement the set_property > hook in the hdmi driver. If you didn't have the wrapper, you just > implement the hook in the hdmi driver. > As I mentioned earlier, display_ops is needed to have no any dependency of drm framework directly like below, DRM Framework | Exynos DRM Framework / | \ Real device drivers In particular, in case of ARM based DRM drivers with separated devices, I still tend to think it's better design than that device drivers implement the connector callbacks directly, but I will try to consider what is the better way. Anyway, can you fix the build error to vidi module? I cannot merge your re-factoring patch set to exynos-drm-next. Thanks, Inki Dae > Sean > > >> Anyway, if there are any reasons that Exynos >> drm based drivers should necessary include drm framework header >> directly, I will agree it. >> >> Thanks, >> Inki Dae >> >>> >>> Sean >>> >>> >>>> Thanks for your opinions. >>>> Inki Dae >>>> >>>>> Stéphane >>>>> >>>>> >>>>>> >>>>>> Thanks, >>>>>> Inki Dae >>>>>> >>>>>> > >>>>>> > Stéphane >>>>>> > >>>>>> > >>>>>> >> >>>>>> >> If so, That would really be >>>>>> >> horrible. :( >>>>>> >> >>>>>> > >>>>>> > >>>>>> >> >>>>>> >> Please, know that only Exynos drm framework, _not device drivers_, has >>>>>> >> all dependencies of drm framework, and also I know that other ARM >>>>>> >> based drm drivers are using same way. >>>>>> >> >>>>>> >> Thanks, >>>>>> >> Inki Dae >>>>>> >> >>>>>> >> > >>>>>> >> > Stéphane >>>>>> >> > >>>>>> >> >> >>>>>> >> >> >>>>>> >> >> > There are a bunch of real bugs that we've found as a result of >>>>>> >> >> > having >>>>>> >> >> > these abstraction layers. Take, for example, dpms. Before this >>>>>> >> >> > patchset, dpms for fimd was being tracked separately in fimd >>>>>> >> >> > driver, >>>>>> >> >> > exynos_drm_encoder, exynos_drm_crtc, and exynos_drm_connector. >>>>>> >> >> > Furthermore, during suspend, only fimd driver's dpms state was >>>>>> >> >> > updated, so the others were incorrect. There was also this weird >>>>>> >> >> > gymnastics that had to happen when dpms was changed in the encoder >>>>>> >> >> > since it had to walk up to the connector level to change its dpms >>>>>> >> >> > state. If fimd just directly implemented >>>>>> >> >> > drm_crtc/drm_encoder/drm_connector (before dp was moved in), this >>>>>> >> >> > problem wouldn't exist. The same goes for HDMI/mixer. >>>>>> >> >> > >>>>>> >> >> >>>>>> >> >> That is a issue we should take care of by using the independent >>>>>> >> >> layer. >>>>>> >> >> Then, aren't you take care of that well with the re-factoring patch >>>>>> >> >> set? :) It seems that you are outside real point. >>>>>> >> >> >>>>>> >> >> > Take a look at exynos_drm_encoder.c in my tree >>>>>> >> >> > >>>>>> >> >> > >>>>>> >> >> > >>>>>> >> >> > (https://github.com/crseanpaul/exynos-drm-next/blob/linux-next-exynos-staging/drivers/gpu/drm/exynos/exynos_drm_encoder.c), >>>>>> >> >> > what does it do that's useful to abstract? All that it does is >>>>>> >> >> > just >>>>>> >> >> > call display ops, it's completely useless. The same is true for >>>>>> >> >> > exynos_drm_connector, it's just dead weight. There is some useful >>>>>> >> >> > stuff in exynos_drm_crtc for page flipping, that would be better >>>>>> >> >> > served as a helper library, though. >>>>>> >> >> > >>>>>> >> >> >> The abstraction layer you mentioned also means a common spot. >>>>>> >> >> >> Another one, you patch also makes each sub driver have strongly >>>>>> >> >> >> dependency >>>>>> >> >> >> of drm framework. So how we can support existing backlight and >>>>>> >> >> >> lcd >>>>>> >> >> >> class >>>>>> >> >> >> based lcd panel drivers if the connector is implemented in each >>>>>> >> >> >> device >>>>>> >> >> >> driver later? the drm header files should be included in >>>>>> >> >> >> drivers/video/backlight/xxx_lcd.c? >>>>>> >> >> >> >>>>>> >> >> > >>>>>> >> >> > drm_bridge or drm_panel seem like good candidates for this. >>>>>> >> >> > >>>>>> >> >> >>>>>> >> >> Yes, exynos_drm_display could be replaced with drm_panel later if >>>>>> >> >> the >>>>>> >> >> drm_panel can be merged to mainline. >>>>>> >> >> >>>>>> >> >> > >>>>>> >> >> >> And, I will introduce a new framework to support existing lcd >>>>>> >> >> >> panel >>>>>> >> >> >> drivers >>>>>> >> >> >> and display bus drivers soon; as of now for Exynos drm, and the >>>>>> >> >> >> framework is >>>>>> >> >> >> being tested internally. With this framework, encoder and >>>>>> >> >> >> connector >>>>>> >> >> >> will be >>>>>> >> >> >> created when lcd panel or display bus driver such as eDP is >>>>>> >> >> >> probed: >>>>>> >> >> >> it >>>>>> >> >> >> doesn’t really need to create encoder and connector in advance if >>>>>> >> >> >> lcd >>>>>> >> >> >> panel >>>>>> >> >> >> or display bus driver isn't probed yet. Regardless of crtc, and >>>>>> >> >> >> encoder >>>>>> >> >> >> and >>>>>> >> >> >> connector creation order, when last one is created, crtc and >>>>>> >> >> >> connector >>>>>> >> >> >> will >>>>>> >> >> >> be connected each other. And exynos_drm_display could be >>>>>> >> >> >> implemented >>>>>> >> >> >> in >>>>>> >> >> >> other frameworks if we have common structure for display device >>>>>> >> >> >> driver. >>>>>> >> >> >> And >>>>>> >> >> >> also the framework will support lvds driver according to Linux >>>>>> >> >> >> device >>>>>> >> >> >> driver >>>>>> >> >> >> model. >>>>>> >> >> >> >>>>>> >> >> > >>>>>> >> >> > I don't really follow what you're trying to do here, but I think >>>>>> >> >> > we >>>>>> >> >> > should be moving in the direction of fewer abstractions in the >>>>>> >> >> > exynos >>>>>> >> >> > driver, not more :) >>>>>> >> >> > >>>>>> >> >> >>>>>> >> >> Not abstraction layer, just a bridge for connecting crtc and its >>>>>> >> >> corresponding encoder/connector, and lvds regardless of creation >>>>>> >> >> order, and for connecting drm connector and other framework based >>>>>> >> >> display ops such as drm_panel later. >>>>>> >> >> >>>>>> >> >> > Sean >>>>>> >> >> > >>>>>> >> >> > >>>>>> >> >> > >>>>>> >> >> >> Thanks, >>>>>> >> >> >> Inki Dae >>>>>> >> >> >> >>>>>> >> >> >>> Sean >>>>>> >> >> >>> >>>>>> >> >> >>> >>> >>>>>> >> >> >>> >>> > And another one, the patch 6 passes manager object to >>>>>> >> >> >>> >>> > manager_ops, >>>>>> >> >> >>> and >>>>>> >> >> >>> >>> for >>>>>> >> >> >>> >>> > this, you made the manager object to be set to driver >>>>>> >> >> >>> >>> > data; >>>>>> >> >> >>> >>> > platform_set_drvdata(pdev, &manager). That isn't >>>>>> >> >> >>> >>> > reasonable. >>>>>> >> >> >>> Generally, >>>>>> >> >> >>> >>> > driver_data would point to device driver's context object. >>>>>> >> >> >>> >>> > >>>>>> >> >> >>> >>> >>>>>> >> >> >>> >>> I'm not sure why this isn't reasonable, but it's a moot >>>>>> >> >> >>> >>> point. >>>>>> >> >> >>> >>> The >>>>>> >> >> >>> >>> driver data is only used up until we get rid of the pm ops, >>>>>> >> >> >>> >>> it >>>>>> >> >> >>> >>> needn't >>>>>> >> >> >>> >>> be set at all once things go through dpms. >>>>>> >> >> >>> >>> >>>>>> >> >> >>> >> >>>>>> >> >> >>> >> Generally, device drivers can call its own internal >>>>>> >> >> >>> >> functions, >>>>>> >> >> >>> >> and >>>>>> >> >> >>> >> they >>>>>> >> >> >>> will >>>>>> >> >> >>> >> call that functions with ctx. However, if you set manager to >>>>>> >> >> >>> driver_data >>>>>> >> >> >>> >> then that functions should be called with manager object and >>>>>> >> >> >>> >> also >>>>>> >> >> >>> internally >>>>>> >> >> >>> >> that functions should get ctx from the manager object. What >>>>>> >> >> >>> >> is >>>>>> >> >> >>> >> the >>>>>> >> >> >>> purpose >>>>>> >> >> >>> >> of manager? Do you think it's reasonable? >>>>>> >> >> >>> >> >>>>>> >> >> >>> > >>>>>> >> >> >>> > So, to avoid setting the manager as the drvdata, we could >>>>>> >> >> >>> > implement >>>>>> >> >> >>> > something like fimd_dpms_ctx(ctx, mode) that takes ctx and the >>>>>> >> >> >>> > manager >>>>>> >> >> >>> > callback calls it fimd_dpms(mgr, mode) { ctx = mgr->ctx; >>>>>> >> >> >>> > fimd_dpms_ctx(ctx, mode); }. Alternatively, you can save a >>>>>> >> >> >>> > pointer >>>>>> >> >> >>> > to >>>>>> >> >> >>> > mgr in ctx, but that creates a circular link between the two. >>>>>> >> >> >>> > IMO, >>>>>> >> >> >>> > both of those solutions suck :) >>>>>> >> >> >>> > >>>>>> >> >> >>> > I'd much rather just set drvdata to the manager and call the >>>>>> >> >> >>> > hook >>>>>> >> >> >>> > directly. Like I said earlier, this is just a temporary step >>>>>> >> >> >>> > since >>>>>> >> >> >>> > we >>>>>> >> >> >>> > remove these pm ops later in the patch series. >>>>>> >> >> >>> > >>>>>> >> >> >>> > Sean >>>>>> >> >> >>> > >>>>>> >> >> >>> > >>>>>> >> >> >>> >> Anyway, I'd like to say really sorry about inconvenient >>>>>> >> >> >>> >> again. >>>>>> >> >> >>> >> So I >>>>>> >> >> >>> will fix >>>>>> >> >> >>> >> it. >>>>>> >> >> >>> >> >>>>>> >> >> >>> >> Thanks, >>>>>> >> >> >>> >> Inki Dae >>>>>> >> >> >>> >> >>>>>> >> >> >>> >>> Sean >>>>>> >> >> >>> >>> >>>>>> >> >> >>> >>> >>>>>> >> >> >>> >>> > Thanks, >>>>>> >> >> >>> >>> > Inki Dae >>>>>> >> >> >>> >>> > >>>>>> >> >> >>> >> >>>>>> >> >> >> >>>>>> >> >> > _______________________________________________ >>>>>> >> >> > dri-devel mailing list >>>>>> >> >> > dri-devel@lists.freedesktop.org >>>>>> >> >> > http://lists.freedesktop.org/mailman/listinfo/dri-devel >>>>>> >> >> _______________________________________________ >>>>>> >> >> dri-devel mailing list >>>>>> >> >> dri-devel@lists.freedesktop.org >>>>>> >> >> http://lists.freedesktop.org/mailman/listinfo/dri-devel >>>>>> >> > >>>>>> >> > >>>>>> >> > >>>>>> >> > _______________________________________________ >>>>>> >> > dri-devel mailing list >>>>>> >> > dri-devel@lists.freedesktop.org >>>>>> >> > http://lists.freedesktop.org/mailman/listinfo/dri-devel >>>>>> >> > >>>>>> > >>>>>> > >>>>>> > >>>>>> > _______________________________________________ >>>>>> > dri-devel mailing list >>>>>> > dri-devel@lists.freedesktop.org >>>>>> > http://lists.freedesktop.org/mailman/listinfo/dri-devel >>>>>> > >>>>> >>>>> >>>>> >>>>> _______________________________________________ >>>>> dri-devel mailing list >>>>> dri-devel@lists.freedesktop.org >>>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel >>>>> >>>> _______________________________________________ >>>> dri-devel mailing list >>>> dri-devel@lists.freedesktop.org >>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel >>> _______________________________________________ >>> dri-devel mailing list >>> dri-devel@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel

diff --git a/drivers/gpu/drm/exynos/exynos_drm_connector.c b/drivers/gpu/drm/exynos/exynos_drm_connector.c index ca270e2..9a16dbe 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_connector.c +++ b/drivers/gpu/drm/exynos/exynos_drm_connector.c @@ -23,26 +23,20 @@ drm_connector) struct exynos_drm_connector { - struct drm_connector drm_connector; - uint32_t encoder_id; - struct exynos_drm_manager *manager; + struct drm_connector drm_connector; + uint32_t encoder_id; + struct exynos_drm_display *display; }; static int exynos_drm_connector_get_modes(struct drm_connector *connector) { struct exynos_drm_connector *exynos_connector = to_exynos_connector(connector); - struct exynos_drm_manager *manager = exynos_connector->manager; - struct exynos_drm_display_ops *display_ops = manager->display_ops; + struct exynos_drm_display *display = exynos_connector->display; struct edid *edid = NULL; unsigned int count = 0; int ret; - if (!display_ops) { - DRM_DEBUG_KMS("display_ops is null.\n"); - return 0; - } - /* * if get_edid() exists then get_edid() callback of hdmi side * is called to get edid data through i2c interface else @@ -51,8 +45,8 @@ static int exynos_drm_connector_get_modes(struct drm_connector *connector) * P.S. in case of lcd panel, count is always 1 if success * because lcd panel has only one mode. */ - if (display_ops->get_edid) { - edid = display_ops->get_edid(manager->dev, connector); + if (display->ops->get_edid) { + edid = display->ops->get_edid(display, connector); if (IS_ERR_OR_NULL(edid)) { ret = PTR_ERR(edid); edid = NULL; @@ -75,8 +69,8 @@ static int exynos_drm_connector_get_modes(struct drm_connector *connector) return 0; } - if (display_ops->get_panel) - panel = display_ops->get_panel(manager->dev); + if (display->ops->get_panel) + panel = display->ops->get_panel(display); else { drm_mode_destroy(connector->dev, mode); return 0; @@ -105,14 +99,13 @@ static int exynos_drm_connector_mode_valid(struct drm_connector *connector, { struct exynos_drm_connector *exynos_connector = to_exynos_connector(connector); - struct exynos_drm_manager *manager = exynos_connector->manager; - struct exynos_drm_display_ops *display_ops = manager->display_ops; + struct exynos_drm_display *display = exynos_connector->display; int ret = MODE_BAD; DRM_DEBUG_KMS("%s\n", __FILE__); - if (display_ops && display_ops->check_mode) - if (!display_ops->check_mode(manager->dev, mode)) + if (display->ops->check_mode) + if (!display->ops->check_mode(display, mode)) ret = MODE_OK; return ret; @@ -151,8 +144,7 @@ static int exynos_drm_connector_fill_modes(struct drm_connector *connector, { struct exynos_drm_connector *exynos_connector = to_exynos_connector(connector); - struct exynos_drm_manager *manager = exynos_connector->manager; - struct exynos_drm_manager_ops *ops = manager->ops; + struct exynos_drm_display *display = exynos_connector->display; unsigned int width, height; width = max_width; @@ -162,8 +154,8 @@ static int exynos_drm_connector_fill_modes(struct drm_connector *connector, * if specific driver want to find desired_mode using maxmum * resolution then get max width and height from that driver. */ - if (ops && ops->get_max_resol) - ops->get_max_resol(manager, &width, &height); + if (display->ops->get_max_resol) + display->ops->get_max_resol(display, &width, &height); return drm_helper_probe_single_connector_modes(connector, width, height); @@ -175,13 +167,11 @@ exynos_drm_connector_detect(struct drm_connector *connector, bool force) { struct exynos_drm_connector *exynos_connector = to_exynos_connector(connector); - struct exynos_drm_manager *manager = exynos_connector->manager; - struct exynos_drm_display_ops *display_ops = - manager->display_ops; + struct exynos_drm_display *display = exynos_connector->display; enum drm_connector_status status = connector_status_disconnected; - if (display_ops && display_ops->is_connected) { - if (display_ops->is_connected(manager->dev)) + if (display->ops->is_connected) { + if (display->ops->is_connected(display)) status = connector_status_connected; else status = connector_status_disconnected; @@ -211,7 +201,7 @@ struct drm_connector *exynos_drm_connector_create(struct drm_device *dev, struct drm_encoder *encoder) { struct exynos_drm_connector *exynos_connector; - struct exynos_drm_manager *manager = exynos_drm_get_manager(encoder); + struct exynos_drm_display *display = exynos_drm_get_display(encoder); struct drm_connector *connector; int type; int err; @@ -222,7 +212,7 @@ struct drm_connector *exynos_drm_connector_create(struct drm_device *dev, connector = &exynos_connector->drm_connector; - switch (manager->display_ops->type) { + switch (display->type) { case EXYNOS_DISPLAY_TYPE_HDMI: type = DRM_MODE_CONNECTOR_HDMIA; connector->interlace_allowed = true; @@ -245,7 +235,7 @@ struct drm_connector *exynos_drm_connector_create(struct drm_device *dev, goto err_connector; exynos_connector->encoder_id = encoder->base.id; - exynos_connector->manager = manager; + exynos_connector->display = display; connector->dpms = DRM_MODE_DPMS_OFF; connector->encoder = encoder; diff --git a/drivers/gpu/drm/exynos/exynos_drm_core.c b/drivers/gpu/drm/exynos/exynos_drm_core.c index 08ca4f9..e76098d 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_core.c +++ b/drivers/gpu/drm/exynos/exynos_drm_core.c @@ -16,11 +16,14 @@ #include <drm/drmP.h> #include <drm/bridge/ptn3460.h> #include "exynos_drm_drv.h" +#include "exynos_drm_crtc.h" #include "exynos_drm_encoder.h" #include "exynos_drm_connector.h" #include "exynos_drm_fbdev.h" static LIST_HEAD(exynos_drm_subdrv_list); +static LIST_HEAD(exynos_drm_manager_list); +static LIST_HEAD(exynos_drm_display_list); struct bridge_init { struct i2c_client *client; @@ -57,24 +60,28 @@ static int exynos_drm_attach_lcd_bridge(struct drm_device *dev, } static int exynos_drm_create_enc_conn(struct drm_device *dev, - struct exynos_drm_subdrv *subdrv) + struct exynos_drm_display *display) { struct drm_encoder *encoder; struct drm_connector *connector; + struct exynos_drm_manager *manager; int ret; + unsigned long possible_crtcs = 0; - subdrv->manager->dev = subdrv->dev; + /* Find possible crtcs for this display */ + list_for_each_entry(manager, &exynos_drm_manager_list, list) + if (manager->type == display->type) + possible_crtcs |= 1 << manager->pipe; /* create and initialize a encoder for this sub driver. */ - encoder = exynos_drm_encoder_create(dev, subdrv->manager, - (1 << MAX_CRTC) - 1); + encoder = exynos_drm_encoder_create(dev, display, possible_crtcs); if (!encoder) { DRM_ERROR("failed to create encoder\n"); return -EFAULT; } - subdrv->encoder = encoder; + display->encoder = encoder; - if (subdrv->manager->display_ops->type == EXYNOS_DISPLAY_TYPE_LCD) { + if (display->type == EXYNOS_DISPLAY_TYPE_LCD) { ret = exynos_drm_attach_lcd_bridge(dev, encoder); if (ret) return 0; @@ -91,7 +98,7 @@ static int exynos_drm_create_enc_conn(struct drm_device *dev, goto err_destroy_encoder; } - subdrv->connector = connector; + display->connector = connector; return 0; @@ -100,21 +107,6 @@ err_destroy_encoder: return ret; } -static void exynos_drm_destroy_enc_conn(struct exynos_drm_subdrv *subdrv) -{ - if (subdrv->encoder) { - struct drm_encoder *encoder = subdrv->encoder; - encoder->funcs->destroy(encoder); - subdrv->encoder = NULL; - } - - if (subdrv->connector) { - struct drm_connector *connector = subdrv->connector; - connector->funcs->destroy(connector); - subdrv->connector = NULL; - } -} - static int exynos_drm_subdrv_probe(struct drm_device *dev, struct exynos_drm_subdrv *subdrv) { @@ -146,10 +138,98 @@ static void exynos_drm_subdrv_remove(struct drm_device *dev, subdrv->remove(dev, subdrv->dev); } +int exynos_drm_initialize_managers(struct drm_device *dev) +{ + struct exynos_drm_manager *manager, *n; + int ret, pipe = 0; + + list_for_each_entry(manager, &exynos_drm_manager_list, list) { + if (manager->ops->initialize) { + ret = manager->ops->initialize(manager, dev, pipe); + if (ret) { + DRM_ERROR("Mgr init [%d] failed with %d\n", + manager->type, ret); + goto err; + } + } + + manager->drm_dev = dev; + manager->pipe = pipe++; + + ret = exynos_drm_crtc_create(manager); + if (ret) { + DRM_ERROR("CRTC create [%d] failed with %d\n", + manager->type, ret); + goto err; + } + } + return 0; + +err: + list_for_each_entry_safe(manager, n, &exynos_drm_manager_list, list) { + if (pipe-- > 0) + exynos_drm_manager_unregister(manager); + else + list_del(&manager->list); + } + return ret; +} + +void exynos_drm_remove_managers(struct drm_device *dev) +{ + struct exynos_drm_manager *manager, *n; + + list_for_each_entry_safe(manager, n, &exynos_drm_manager_list, list) + exynos_drm_manager_unregister(manager); +} + +int exynos_drm_initialize_displays(struct drm_device *dev) +{ + struct exynos_drm_display *display, *n; + int ret, initialized = 0; + + list_for_each_entry(display, &exynos_drm_display_list, list) { + if (display->ops->initialize) { + ret = display->ops->initialize(display, dev); + if (ret) { + DRM_ERROR("Display init [%d] failed with %d\n", + display->type, ret); + goto err; + } + } + + initialized++; + + ret = exynos_drm_create_enc_conn(dev, display); + if (ret) { + DRM_ERROR("Encoder create [%d] failed with %d\n", + display->type, ret); + goto err; + } + } + return 0; + +err: + list_for_each_entry_safe(display, n, &exynos_drm_display_list, list) { + if (initialized-- > 0) + exynos_drm_display_unregister(display); + else + list_del(&display->list); + } + return ret; +} + +void exynos_drm_remove_displays(struct drm_device *dev) +{ + struct exynos_drm_display *display, *n; + + list_for_each_entry_safe(display, n, &exynos_drm_display_list, list) + exynos_drm_display_unregister(display); +} + int exynos_drm_device_register(struct drm_device *dev) { struct exynos_drm_subdrv *subdrv, *n; - unsigned int fine_cnt = 0; int err; if (!dev) @@ -162,30 +242,8 @@ int exynos_drm_device_register(struct drm_device *dev) list_del(&subdrv->list); continue; } - - /* - * if manager is null then it means that this sub driver - * doesn't need encoder and connector. - */ - if (!subdrv->manager) { - fine_cnt++; - continue; - } - - err = exynos_drm_create_enc_conn(dev, subdrv); - if (err) { - DRM_DEBUG("failed to create encoder and connector.\n"); - exynos_drm_subdrv_remove(dev, subdrv); - list_del(&subdrv->list); - continue; - } - - fine_cnt++; } - if (!fine_cnt) - return -EINVAL; - return 0; } EXPORT_SYMBOL_GPL(exynos_drm_device_register); @@ -201,13 +259,44 @@ int exynos_drm_device_unregister(struct drm_device *dev) list_for_each_entry(subdrv, &exynos_drm_subdrv_list, list) { exynos_drm_subdrv_remove(dev, subdrv); - exynos_drm_destroy_enc_conn(subdrv); } return 0; } EXPORT_SYMBOL_GPL(exynos_drm_device_unregister); +int exynos_drm_manager_register(struct exynos_drm_manager *manager) +{ + BUG_ON(!manager->ops); + list_add_tail(&manager->list, &exynos_drm_manager_list); + return 0; +} + +int exynos_drm_manager_unregister(struct exynos_drm_manager *manager) +{ + if (manager->ops->remove) + manager->ops->remove(manager); + + list_del(&manager->list); + return 0; +} + +int exynos_drm_display_register(struct exynos_drm_display *display) +{ + BUG_ON(!display->ops); + list_add_tail(&display->list, &exynos_drm_display_list); + return 0; +} + +int exynos_drm_display_unregister(struct exynos_drm_display *display) +{ + if (display->ops->remove) + display->ops->remove(display); + + list_del(&display->list); + return 0; +} + int exynos_drm_subdrv_register(struct exynos_drm_subdrv *subdrv) { if (!subdrv) diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index ebc0150..347d62d 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c @@ -33,6 +33,7 @@ enum exynos_crtc_mode { * * @drm_crtc: crtc object. * @drm_plane: pointer of private plane object for this crtc + * @manager: the manager associated with this crtc * @pipe: a crtc index created at load() with a new crtc object creation * and the crtc object would be set to private->crtc array * to get a crtc object corresponding to this pipe from private->crtc @@ -46,6 +47,7 @@ enum exynos_crtc_mode { struct exynos_drm_crtc { struct drm_crtc drm_crtc; struct drm_plane *plane; + struct exynos_drm_manager *manager; unsigned int pipe; unsigned int dpms; enum exynos_crtc_mode mode; @@ -56,6 +58,7 @@ struct exynos_drm_crtc { static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode) { struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc); + struct exynos_drm_manager *manager = exynos_crtc->manager; DRM_DEBUG_KMS("crtc[%d] mode[%d]\n", crtc->base.id, mode); @@ -71,7 +74,9 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode) drm_vblank_off(crtc->dev, exynos_crtc->pipe); } - exynos_drm_fn_encoder(crtc, &mode, exynos_drm_encoder_crtc_dpms); + if (manager->ops->dpms) + manager->ops->dpms(manager, mode); + exynos_crtc->dpms = mode; } @@ -83,9 +88,15 @@ static void exynos_drm_crtc_prepare(struct drm_crtc *crtc) static void exynos_drm_crtc_commit(struct drm_crtc *crtc) { struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc); + struct exynos_drm_manager *manager = exynos_crtc->manager; exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON); + exynos_plane_commit(exynos_crtc->plane); + + if (manager->ops->commit) + manager->ops->commit(manager); + exynos_plane_dpms(exynos_crtc->plane, DRM_MODE_DPMS_ON); } @@ -107,7 +118,6 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_plane *plane = exynos_crtc->plane; unsigned int crtc_w; unsigned int crtc_h; - int pipe = exynos_crtc->pipe; int ret; /* @@ -127,8 +137,6 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, plane->crtc = crtc; plane->fb = crtc->fb; - exynos_drm_fn_encoder(crtc, &pipe, exynos_drm_encoder_crtc_pipe); - return 0; } @@ -318,21 +326,24 @@ static void exynos_drm_crtc_attach_mode_property(struct drm_crtc *crtc) drm_object_attach_property(&crtc->base, prop, 0); } -int exynos_drm_crtc_create(struct drm_device *dev, unsigned int nr) +int exynos_drm_crtc_create(struct exynos_drm_manager *manager) { struct exynos_drm_crtc *exynos_crtc; - struct exynos_drm_private *private = dev->dev_private; + struct exynos_drm_private *private = manager->drm_dev->dev_private; struct drm_crtc *crtc; exynos_crtc = kzalloc(sizeof(*exynos_crtc), GFP_KERNEL); if (!exynos_crtc) return -ENOMEM; - exynos_crtc->pipe = nr; - exynos_crtc->dpms = DRM_MODE_DPMS_OFF; init_waitqueue_head(&exynos_crtc->pending_flip_queue); atomic_set(&exynos_crtc->pending_flip, 0); - exynos_crtc->plane = exynos_plane_init(dev, 1 << nr, true); + + exynos_crtc->dpms = DRM_MODE_DPMS_OFF; + exynos_crtc->manager = manager; + exynos_crtc->pipe = manager->pipe; + exynos_crtc->plane = exynos_plane_init(manager->drm_dev, + 1 << manager->pipe, true); if (!exynos_crtc->plane) { kfree(exynos_crtc); return -ENOMEM; @@ -340,9 +351,9 @@ int exynos_drm_crtc_create(struct drm_device *dev, unsigned int nr) crtc = &exynos_crtc->drm_crtc; - private->crtc[nr] = crtc; + private->crtc[manager->pipe] = crtc; - drm_crtc_init(dev, crtc, &exynos_crtc_funcs); + drm_crtc_init(manager->drm_dev, crtc, &exynos_crtc_funcs); drm_crtc_helper_add(crtc, &exynos_crtc_helper_funcs); exynos_drm_crtc_attach_mode_property(crtc); @@ -350,39 +361,41 @@ int exynos_drm_crtc_create(struct drm_device *dev, unsigned int nr) return 0; } -int exynos_drm_crtc_enable_vblank(struct drm_device *dev, int crtc) +int exynos_drm_crtc_enable_vblank(struct drm_device *dev, int pipe) { struct exynos_drm_private *private = dev->dev_private; struct exynos_drm_crtc *exynos_crtc = - to_exynos_crtc(private->crtc[crtc]); + to_exynos_crtc(private->crtc[pipe]); + struct exynos_drm_manager *manager = exynos_crtc->manager; if (exynos_crtc->dpms != DRM_MODE_DPMS_ON) return -EPERM; - exynos_drm_fn_encoder(private->crtc[crtc], &crtc, - exynos_drm_enable_vblank); + if (manager->ops->enable_vblank) + manager->ops->enable_vblank(manager); return 0; } -void exynos_drm_crtc_disable_vblank(struct drm_device *dev, int crtc) +void exynos_drm_crtc_disable_vblank(struct drm_device *dev, int pipe) { struct exynos_drm_private *private = dev->dev_private; struct exynos_drm_crtc *exynos_crtc = - to_exynos_crtc(private->crtc[crtc]); + to_exynos_crtc(private->crtc[pipe]); + struct exynos_drm_manager *manager = exynos_crtc->manager; if (exynos_crtc->dpms != DRM_MODE_DPMS_ON) return; - exynos_drm_fn_encoder(private->crtc[crtc], &crtc, - exynos_drm_disable_vblank); + if (manager->ops->disable_vblank) + manager->ops->disable_vblank(manager); } -void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int crtc) +void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int pipe) { struct exynos_drm_private *dev_priv = dev->dev_private; struct drm_pending_vblank_event *e, *t; - struct drm_crtc *drm_crtc = dev_priv->crtc[crtc]; + struct drm_crtc *drm_crtc = dev_priv->crtc[pipe]; struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(drm_crtc); unsigned long flags; @@ -391,15 +404,71 @@ void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int crtc) list_for_each_entry_safe(e, t, &dev_priv->pageflip_event_list, base.link) { /* if event's pipe isn't same as crtc then ignore it. */ - if (crtc != e->pipe) + if (pipe != e->pipe) continue; list_del(&e->base.link); drm_send_vblank_event(dev, -1, e); - drm_vblank_put(dev, crtc); + drm_vblank_put(dev, pipe); atomic_set(&exynos_crtc->pending_flip, 0); wake_up(&exynos_crtc->pending_flip_queue); } spin_unlock_irqrestore(&dev->event_lock, flags); } + +void exynos_drm_crtc_plane_mode_set(struct drm_crtc *crtc, + struct exynos_drm_overlay *overlay) +{ + struct exynos_drm_manager *manager = to_exynos_crtc(crtc)->manager; + + if (manager->ops->win_mode_set) + manager->ops->win_mode_set(manager, overlay); +} + +void exynos_drm_crtc_plane_commit(struct drm_crtc *crtc, int zpos) +{ + struct exynos_drm_manager *manager = to_exynos_crtc(crtc)->manager; + + if (manager->ops->win_commit) + manager->ops->win_commit(manager, zpos); +} + +void exynos_drm_crtc_plane_enable(struct drm_crtc *crtc, int zpos) +{ + struct exynos_drm_manager *manager = to_exynos_crtc(crtc)->manager; + + if (manager->ops->win_enable) + manager->ops->win_enable(manager, zpos); +} + +void exynos_drm_crtc_plane_disable(struct drm_crtc *crtc, int zpos) +{ + struct exynos_drm_manager *manager = to_exynos_crtc(crtc)->manager; + + if (manager->ops->win_disable) + manager->ops->win_disable(manager, zpos); +} + +void exynos_drm_crtc_complete_scanout(struct drm_framebuffer *fb) +{ + struct exynos_drm_manager *manager; + struct drm_device *dev = fb->dev; + struct drm_crtc *crtc; + + /* + * make sure that overlay data are updated to real hardware + * for all encoders. + */ + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { + manager = to_exynos_crtc(crtc)->manager; + + /* + * wait for vblank interrupt + * - this makes sure that overlay data are updated to + * real hardware. + */ + if (manager->ops->wait_for_vblank) + manager->ops->wait_for_vblank(manager); + } +} diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.h b/drivers/gpu/drm/exynos/exynos_drm_crtc.h index 3e197e6..c27b66c 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.h +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.h @@ -15,9 +15,21 @@ #ifndef _EXYNOS_DRM_CRTC_H_ #define _EXYNOS_DRM_CRTC_H_ -int exynos_drm_crtc_create(struct drm_device *dev, unsigned int nr); -int exynos_drm_crtc_enable_vblank(struct drm_device *dev, int crtc); -void exynos_drm_crtc_disable_vblank(struct drm_device *dev, int crtc); -void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int crtc); +struct drm_device; +struct drm_crtc; +struct exynos_drm_manager; +struct exynos_drm_overlay; + +int exynos_drm_crtc_create(struct exynos_drm_manager *manager); +int exynos_drm_crtc_enable_vblank(struct drm_device *dev, int pipe); +void exynos_drm_crtc_disable_vblank(struct drm_device *dev, int pipe); +void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int pipe); +void exynos_drm_crtc_complete_scanout(struct drm_framebuffer *fb); + +void exynos_drm_crtc_plane_mode_set(struct drm_crtc *crtc, + struct exynos_drm_overlay *overlay); +void exynos_drm_crtc_plane_commit(struct drm_crtc *crtc, int zpos); +void exynos_drm_crtc_plane_enable(struct drm_crtc *crtc, int zpos); +void exynos_drm_crtc_plane_disable(struct drm_crtc *crtc, int zpos); #endif diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index 617748e..250903c 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -72,15 +72,9 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags) exynos_drm_mode_config_init(dev); - /* - * EXYNOS4 is enough to have two CRTCs and each crtc would be used - * without dependency of hardware. - */ - for (nr = 0; nr < MAX_CRTC; nr++) { - ret = exynos_drm_crtc_create(dev, nr); - if (ret) - goto err_release_iommu_mapping; - } + ret = exynos_drm_initialize_managers(dev); + if (ret) + goto err_mode_config_cleanup; for (nr = 0; nr < MAX_PLANE; nr++) { struct drm_plane *plane; @@ -88,12 +82,16 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags) plane = exynos_plane_init(dev, possible_crtcs, false); if (!plane) - goto err_release_iommu_mapping; + goto err_manager_cleanup; } + ret = exynos_drm_initialize_displays(dev); + if (ret) + goto err_manager_cleanup; + ret = drm_vblank_init(dev, MAX_CRTC); if (ret) - goto err_release_iommu_mapping; + goto err_display_cleanup; /* * probe sub drivers such as display controller and hdmi driver, @@ -125,7 +123,12 @@ err_drm_device: exynos_drm_device_unregister(dev); err_vblank: drm_vblank_cleanup(dev); -err_release_iommu_mapping: +err_display_cleanup: + exynos_drm_remove_displays(dev); +err_manager_cleanup: + exynos_drm_remove_managers(dev); +err_mode_config_cleanup: + drm_mode_config_cleanup(dev); drm_release_iommu_mapping(dev); err_crtc: drm_mode_config_cleanup(dev); @@ -140,6 +143,8 @@ static int exynos_drm_unload(struct drm_device *dev) exynos_drm_device_unregister(dev); drm_vblank_cleanup(dev); drm_kms_helper_poll_fini(dev); + exynos_drm_remove_displays(dev); + exynos_drm_remove_managers(dev); drm_mode_config_cleanup(dev); drm_release_iommu_mapping(dev); diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h index e31c088..dc730e5 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h @@ -122,34 +122,68 @@ struct exynos_drm_overlay { * Exynos DRM Display Structure. * - this structure is common to analog tv, digital tv and lcd panel. * - * @type: one of EXYNOS_DISPLAY_TYPE_LCD and HDMI. * @initialize: initializes the display with drm_dev + * @remove: cleans up the display for removal * @is_connected: check for that display is connected or not. + * @get_max_resol: get maximum resolution to specific hardware. * @get_edid: get edid modes from display driver. * @get_panel: get panel object from display driver. + * @mode_fixup: fix mode data comparing to hw specific display mode. + * @mode_set: convert drm_display_mode to hw specific display mode and + * would be called by encoder->mode_set(). * @check_mode: check if mode is valid or not. * @dpms: display device on or off. + * @commit: apply changes to hw */ +struct exynos_drm_display; struct exynos_drm_display_ops { + int (*initialize)(struct exynos_drm_display *display, + struct drm_device *drm_dev); + void (*remove)(struct exynos_drm_display *display); + bool (*is_connected)(struct exynos_drm_display *display); + void (*get_max_resol)(struct exynos_drm_display *display, + unsigned int *width, + unsigned int *height); + struct edid *(*get_edid)(struct exynos_drm_display *display, + struct drm_connector *connector); + void *(*get_panel)(struct exynos_drm_display *display); + void (*mode_fixup)(struct exynos_drm_display *display, + struct drm_connector *connector, + const struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode); + void (*mode_set)(struct exynos_drm_display *display, + struct drm_display_mode *mode); + int (*check_mode)(struct exynos_drm_display *display, + struct drm_display_mode *mode); + void (*dpms)(struct exynos_drm_display *display, int mode); + void (*commit)(struct exynos_drm_display *display); +}; + +/* + * Exynos drm display structure, maps 1:1 with an encoder/connector + * + * @list: the list entry for this manager + * @type: one of EXYNOS_DISPLAY_TYPE_LCD and HDMI. + * @encoder: encoder object this display maps to + * @connector: connector object this display maps to + * @ops: pointer to callbacks for exynos drm specific functionality + * @ctx: A pointer to the display's implementation specific context + */ +struct exynos_drm_display { + struct list_head list; enum exynos_drm_output_type type; - int (*initialize)(struct device *dev, struct drm_device *drm_dev); - bool (*is_connected)(struct device *dev); - struct edid *(*get_edid)(struct device *dev, - struct drm_connector *connector); - void *(*get_panel)(struct device *dev); - int (*check_mode)(struct device *dev, struct drm_display_mode *mode); - int (*dpms)(struct device *dev, int mode); + struct drm_encoder *encoder; + struct drm_connector *connector; + struct exynos_drm_display_ops *ops; + void *ctx; }; /* * Exynos drm manager ops * * @initialize: initializes the manager with drm_dev + * @remove: cleans up the manager for removal * @dpms: control device power. - * @mode_fixup: fix mode data comparing to hw specific display mode. - * @mode_set: convert drm_display_mode to hw specific display mode and - * would be called by encoder->mode_set(). - * @get_max_resol: get maximum resolution to specific hardware. * @commit: set current hw specific display mode to hw. * @enable_vblank: specific driver callback for enabling vblank interrupt. * @disable_vblank: specific driver callback for disabling vblank interrupt. @@ -163,15 +197,9 @@ struct exynos_drm_display_ops { struct exynos_drm_manager; struct exynos_drm_manager_ops { int (*initialize)(struct exynos_drm_manager *mgr, - struct drm_device *drm_dev); + struct drm_device *drm_dev, int pipe); + void (*remove)(struct exynos_drm_manager *mgr); void (*dpms)(struct exynos_drm_manager *mgr, int mode); - void (*mode_fixup)(struct exynos_drm_manager *mgr, - struct drm_connector *connector, - const struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode); - void (*mode_set)(struct exynos_drm_manager *mgr, void *mode); - void (*get_max_resol)(struct exynos_drm_manager *mgr, - unsigned int *width, unsigned int *height); void (*commit)(struct exynos_drm_manager *mgr); int (*enable_vblank)(struct exynos_drm_manager *mgr); void (*disable_vblank)(struct exynos_drm_manager *mgr); @@ -184,25 +212,21 @@ struct exynos_drm_manager_ops { }; /* - * Exynos drm common manager structure. + * Exynos drm common manager structure, maps 1:1 with a crtc * - * @dev: pointer to device object for subdrv device driver. - * sub drivers such as display controller or hdmi driver, - * have their own device object. - * @ops: pointer to callbacks for exynos drm specific framebuffer. - * these callbacks should be set by specific drivers such fimd - * or hdmi driver and are used to control hardware global registers. - * @display: pointer to callbacks for exynos drm specific framebuffer. - * these callbacks should be set by specific drivers such fimd - * or hdmi driver and are used to control display devices such as - * analog tv, digital tv and lcd panel and also get timing data for them. + * @list: the list entry for this manager + * @type: one of EXYNOS_DISPLAY_TYPE_LCD and HDMI. + * @drm_dev: pointer to the drm device + * @pipe: the pipe number for this crtc/manager + * @ops: pointer to callbacks for exynos drm specific functionality * @ctx: A pointer to the manager's implementation specific context */ struct exynos_drm_manager { - struct device *dev; + struct list_head list; + enum exynos_drm_output_type type; + struct drm_device *drm_dev; int pipe; struct exynos_drm_manager_ops *ops; - struct exynos_drm_display_ops *display_ops; void *ctx; }; @@ -267,14 +291,11 @@ struct exynos_drm_private { * by probe callback. * @open: this would be called with drm device file open. * @close: this would be called with drm device file close. - * @encoder: encoder object owned by this sub driver. - * @connector: connector object owned by this sub driver. */ struct exynos_drm_subdrv { struct list_head list; struct device *dev; struct drm_device *drm_dev; - struct exynos_drm_manager *manager; int (*probe)(struct drm_device *drm_dev, struct device *dev); void (*remove)(struct drm_device *drm_dev, struct device *dev); @@ -282,9 +303,6 @@ struct exynos_drm_subdrv { struct drm_file *file); void (*close)(struct drm_device *drm_dev, struct device *dev, struct drm_file *file); - - struct drm_encoder *encoder; - struct drm_connector *connector; }; /* @@ -299,6 +317,16 @@ int exynos_drm_device_register(struct drm_device *dev); */ int exynos_drm_device_unregister(struct drm_device *dev); +int exynos_drm_initialize_managers(struct drm_device *dev); +void exynos_drm_remove_managers(struct drm_device *dev); +int exynos_drm_initialize_displays(struct drm_device *dev); +void exynos_drm_remove_displays(struct drm_device *dev); + +int exynos_drm_manager_register(struct exynos_drm_manager *manager); +int exynos_drm_manager_unregister(struct exynos_drm_manager *manager); +int exynos_drm_display_register(struct exynos_drm_display *display); +int exynos_drm_display_unregister(struct exynos_drm_display *display); + /* * this function would be called by sub drivers such as display controller * or hdmi driver to register this sub driver object to exynos drm driver diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c b/drivers/gpu/drm/exynos/exynos_drm_encoder.c index efe4e60..d4ae664 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.c @@ -26,24 +26,23 @@ * exynos specific encoder structure. * * @drm_encoder: encoder object. - * @manager: specific encoder has its own manager to control a hardware - * appropriately and we can access a hardware drawing on this manager. + * @display: the display structure that maps to this encoder */ struct exynos_drm_encoder { struct drm_crtc *old_crtc; struct drm_encoder drm_encoder; - struct exynos_drm_manager *manager; + struct exynos_drm_display *display; }; static void exynos_drm_encoder_dpms(struct drm_encoder *encoder, int mode) { - struct exynos_drm_manager *manager = exynos_drm_get_manager(encoder); - struct exynos_drm_display_ops *display_ops = manager->display_ops; + struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder); + struct exynos_drm_display *display = exynos_encoder->display; DRM_DEBUG_KMS("encoder dpms: %d\n", mode); - if (display_ops && display_ops->dpms) - display_ops->dpms(manager->ctx, mode); + if (display->ops->dpms) + display->ops->dpms(display, mode); } static bool @@ -52,15 +51,17 @@ exynos_drm_encoder_mode_fixup(struct drm_encoder *encoder, struct drm_display_mode *adjusted_mode) { struct drm_device *dev = encoder->dev; + struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder); + struct exynos_drm_display *display = exynos_encoder->display; struct drm_connector *connector; - struct exynos_drm_manager *manager = exynos_drm_get_manager(encoder); - struct exynos_drm_manager_ops *manager_ops = manager->ops; list_for_each_entry(connector, &dev->mode_config.connector_list, head) { - if (connector->encoder == encoder) - if (manager_ops && manager_ops->mode_fixup) - manager_ops->mode_fixup(manager, connector, - mode, adjusted_mode); + if (connector->encoder != encoder) + continue; + + if (display->ops->mode_fixup) + display->ops->mode_fixup(display, connector, mode, + adjusted_mode); } return true; @@ -102,8 +103,7 @@ static void exynos_drm_encoder_mode_set(struct drm_encoder *encoder, { struct drm_device *dev = encoder->dev; struct drm_connector *connector; - struct exynos_drm_manager *manager; - struct exynos_drm_manager_ops *manager_ops; + struct exynos_drm_display *display; list_for_each_entry(connector, &dev->mode_config.connector_list, head) { if (connector->encoder == encoder) { @@ -123,11 +123,11 @@ static void exynos_drm_encoder_mode_set(struct drm_encoder *encoder, encoder->crtc); } - manager = exynos_drm_get_manager(encoder); - manager_ops = manager->ops; + display = exynos_encoder->display; - if (manager_ops && manager_ops->mode_set) - manager_ops->mode_set(manager, adjusted_mode); + if (display->ops->mode_set) + display->ops->mode_set(display, + adjusted_mode); exynos_encoder->old_crtc = encoder->crtc; } @@ -142,39 +142,15 @@ static void exynos_drm_encoder_prepare(struct drm_encoder *encoder) static void exynos_drm_encoder_commit(struct drm_encoder *encoder) { struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder); - struct exynos_drm_manager *manager = exynos_encoder->manager; - struct exynos_drm_manager_ops *manager_ops = manager->ops; - - if (manager_ops && manager_ops->commit) - manager_ops->commit(manager); -} + struct exynos_drm_display *display = exynos_encoder->display; -void exynos_drm_encoder_complete_scanout(struct drm_framebuffer *fb) -{ - struct exynos_drm_encoder *exynos_encoder; - struct exynos_drm_manager_ops *ops; - struct drm_device *dev = fb->dev; - struct drm_encoder *encoder; + if (display->ops->dpms) + display->ops->dpms(display, DRM_MODE_DPMS_ON); - /* - * make sure that overlay data are updated to real hardware - * for all encoders. - */ - list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) { - exynos_encoder = to_exynos_encoder(encoder); - ops = exynos_encoder->manager->ops; - - /* - * wait for vblank interrupt - * - this makes sure that overlay data are updated to - * real hardware. - */ - if (ops->wait_for_vblank) - ops->wait_for_vblank(exynos_encoder->manager); - } + if (display->ops->commit) + display->ops->commit(display); } - static void exynos_drm_encoder_disable(struct drm_encoder *encoder) { struct drm_plane *plane; @@ -200,10 +176,7 @@ static struct drm_encoder_helper_funcs exynos_encoder_helper_funcs = { static void exynos_drm_encoder_destroy(struct drm_encoder *encoder) { - struct exynos_drm_encoder *exynos_encoder = - to_exynos_encoder(encoder); - - exynos_encoder->manager->pipe = -1; + struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder); drm_encoder_cleanup(encoder); kfree(exynos_encoder); @@ -218,13 +191,12 @@ static unsigned int exynos_drm_encoder_clones(struct drm_encoder *encoder) struct drm_encoder *clone; struct drm_device *dev = encoder->dev; struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder); - struct exynos_drm_display_ops *display_ops = - exynos_encoder->manager->display_ops; + struct exynos_drm_display *display = exynos_encoder->display; unsigned int clone_mask = 0; int cnt = 0; list_for_each_entry(clone, &dev->mode_config.encoder_list, head) { - switch (display_ops->type) { + switch (display->type) { case EXYNOS_DISPLAY_TYPE_LCD: case EXYNOS_DISPLAY_TYPE_HDMI: case EXYNOS_DISPLAY_TYPE_VIDI: @@ -248,24 +220,20 @@ void exynos_drm_encoder_setup(struct drm_device *dev) struct drm_encoder * exynos_drm_encoder_create(struct drm_device *dev, - struct exynos_drm_manager *manager, + struct exynos_drm_display *display, unsigned long possible_crtcs) { struct drm_encoder *encoder; struct exynos_drm_encoder *exynos_encoder; - int ret; - if (!manager || !possible_crtcs) - return NULL; - - if (!manager->dev) + if (!possible_crtcs) return NULL; exynos_encoder = kzalloc(sizeof(*exynos_encoder), GFP_KERNEL); if (!exynos_encoder) return NULL; - exynos_encoder->manager = manager; + exynos_encoder->display = display; encoder = &exynos_encoder->drm_encoder; encoder->possible_crtcs = possible_crtcs; @@ -276,174 +244,12 @@ exynos_drm_encoder_create(struct drm_device *dev, drm_encoder_helper_add(encoder, &exynos_encoder_helper_funcs); - if (manager->ops && manager->ops->initialize) { - ret = manager->ops->initialize(manager, dev); - if (ret) { - DRM_ERROR("Manager initialize failed %d\n", ret); - goto error; - } - } - - if (manager->display_ops && manager->display_ops->initialize) { - ret = manager->display_ops->initialize(manager->dev, dev); - if (ret) { - DRM_ERROR("Display initialize failed %d\n", ret); - goto error; - } - } - DRM_DEBUG_KMS("encoder has been created\n"); return encoder; - -error: - exynos_drm_encoder_destroy(&exynos_encoder->drm_encoder); - return NULL; } -struct exynos_drm_manager *exynos_drm_get_manager(struct drm_encoder *encoder) +struct exynos_drm_display *exynos_drm_get_display(struct drm_encoder *encoder) { - return to_exynos_encoder(encoder)->manager; -} - -void exynos_drm_fn_encoder(struct drm_crtc *crtc, void *data, - void (*fn)(struct drm_encoder *, void *)) -{ - struct drm_device *dev = crtc->dev; - struct drm_encoder *encoder; - struct exynos_drm_private *private = dev->dev_private; - struct exynos_drm_manager *manager; - - list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) { - /* - * if crtc is detached from encoder, check pipe, - * otherwise check crtc attached to encoder - */ - if (!encoder->crtc) { - manager = to_exynos_encoder(encoder)->manager; - if (manager->pipe < 0 || - private->crtc[manager->pipe] != crtc) - continue; - } else { - if (encoder->crtc != crtc) - continue; - } - - fn(encoder, data); - } -} - -void exynos_drm_enable_vblank(struct drm_encoder *encoder, void *data) -{ - struct exynos_drm_manager *manager = - to_exynos_encoder(encoder)->manager; - struct exynos_drm_manager_ops *manager_ops = manager->ops; - int crtc = *(int *)data; - - if (manager->pipe != crtc) - return; - - if (manager_ops->enable_vblank) - manager_ops->enable_vblank(manager); -} - -void exynos_drm_disable_vblank(struct drm_encoder *encoder, void *data) -{ - struct exynos_drm_manager *manager = - to_exynos_encoder(encoder)->manager; - struct exynos_drm_manager_ops *manager_ops = manager->ops; - int crtc = *(int *)data; - - if (manager->pipe != crtc) - return; - - if (manager_ops->disable_vblank) - manager_ops->disable_vblank(manager); -} - -void exynos_drm_encoder_crtc_dpms(struct drm_encoder *encoder, void *data) -{ - struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder); - struct exynos_drm_manager *manager = exynos_encoder->manager; - struct exynos_drm_manager_ops *manager_ops = manager->ops; - int mode = *(int *)data; - - if (manager_ops && manager_ops->dpms) - manager_ops->dpms(manager, mode); - - /* - * if this condition is ok then it means that the crtc is already - * detached from encoder and last function for detaching is properly - * done, so clear pipe from manager to prevent repeated call. - */ - if (mode > DRM_MODE_DPMS_ON) { - if (!encoder->crtc) - manager->pipe = -1; - } -} - -void exynos_drm_encoder_crtc_pipe(struct drm_encoder *encoder, void *data) -{ - struct exynos_drm_manager *manager = - to_exynos_encoder(encoder)->manager; - int pipe = *(int *)data; - - /* - * when crtc is detached from encoder, this pipe is used - * to select manager operation - */ - manager->pipe = pipe; -} - -void exynos_drm_encoder_plane_mode_set(struct drm_encoder *encoder, void *data) -{ - struct exynos_drm_manager *manager = - to_exynos_encoder(encoder)->manager; - struct exynos_drm_manager_ops *manager_ops = manager->ops; - struct exynos_drm_overlay *overlay = data; - - if (manager_ops && manager_ops->win_mode_set) - manager_ops->win_mode_set(manager, overlay); -} - -void exynos_drm_encoder_plane_commit(struct drm_encoder *encoder, void *data) -{ - struct exynos_drm_manager *manager = - to_exynos_encoder(encoder)->manager; - struct exynos_drm_manager_ops *manager_ops = manager->ops; - int zpos = DEFAULT_ZPOS; - - if (data) - zpos = *(int *)data; - - if (manager_ops && manager_ops->win_commit) - manager_ops->win_commit(manager, zpos); -} - -void exynos_drm_encoder_plane_enable(struct drm_encoder *encoder, void *data) -{ - struct exynos_drm_manager *manager = - to_exynos_encoder(encoder)->manager; - struct exynos_drm_manager_ops *manager_ops = manager->ops; - int zpos = DEFAULT_ZPOS; - - if (data) - zpos = *(int *)data; - - if (manager_ops && manager_ops->win_enable) - manager_ops->win_enable(manager, zpos); -} - -void exynos_drm_encoder_plane_disable(struct drm_encoder *encoder, void *data) -{ - struct exynos_drm_manager *manager = - to_exynos_encoder(encoder)->manager; - struct exynos_drm_manager_ops *manager_ops = manager->ops; - int zpos = DEFAULT_ZPOS; - - if (data) - zpos = *(int *)data; - - if (manager_ops && manager_ops->win_disable) - manager_ops->win_disable(manager, zpos); + return to_exynos_encoder(encoder)->display; } diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.h b/drivers/gpu/drm/exynos/exynos_drm_encoder.h index 0f3e5e2..b7a1620 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.h +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.h @@ -18,20 +18,8 @@ struct exynos_drm_manager; void exynos_drm_encoder_setup(struct drm_device *dev); struct drm_encoder *exynos_drm_encoder_create(struct drm_device *dev, - struct exynos_drm_manager *mgr, - unsigned long possible_crtcs); -struct exynos_drm_manager * -exynos_drm_get_manager(struct drm_encoder *encoder); -void exynos_drm_fn_encoder(struct drm_crtc *crtc, void *data, - void (*fn)(struct drm_encoder *, void *)); -void exynos_drm_enable_vblank(struct drm_encoder *encoder, void *data); -void exynos_drm_disable_vblank(struct drm_encoder *encoder, void *data); -void exynos_drm_encoder_crtc_dpms(struct drm_encoder *encoder, void *data); -void exynos_drm_encoder_crtc_pipe(struct drm_encoder *encoder, void *data); -void exynos_drm_encoder_plane_mode_set(struct drm_encoder *encoder, void *data); -void exynos_drm_encoder_plane_commit(struct drm_encoder *encoder, void *data); -void exynos_drm_encoder_plane_enable(struct drm_encoder *encoder, void *data); -void exynos_drm_encoder_plane_disable(struct drm_encoder *encoder, void *data); -void exynos_drm_encoder_complete_scanout(struct drm_framebuffer *fb); + struct exynos_drm_display *mgr, + unsigned long possible_crtcs); +struct exynos_drm_display *exynos_drm_get_display(struct drm_encoder *encoder); #endif diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c index ea39e0e..c7c08d0 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c @@ -22,7 +22,7 @@ #include "exynos_drm_fb.h" #include "exynos_drm_gem.h" #include "exynos_drm_iommu.h" -#include "exynos_drm_encoder.h" +#include "exynos_drm_crtc.h" #define to_exynos_fb(x) container_of(x, struct exynos_drm_fb, fb) @@ -71,7 +71,7 @@ static void exynos_drm_fb_destroy(struct drm_framebuffer *fb) unsigned int i; /* make sure that overlay data are updated before relesing fb. */ - exynos_drm_encoder_complete_scanout(fb); + exynos_drm_crtc_complete_scanout(fb); drm_framebuffer_cleanup(fb); diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index b980b05..d2b8ccb 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -105,7 +105,6 @@ struct fimd_win_data { }; struct fimd_context { - struct exynos_drm_subdrv subdrv; struct device *dev; struct drm_device *drm_dev; int irq; @@ -120,6 +119,7 @@ struct fimd_context { u32 vidcon0; u32 vidcon1; bool suspended; + int pipe; struct mutex lock; wait_queue_head_t wait_vsync_queue; atomic_t wait_vsync_event; @@ -147,22 +147,22 @@ static inline struct fimd_driver_data *drm_fimd_get_driver_data( return (struct fimd_driver_data *)of_id->data; } -static bool fimd_display_is_connected(struct device *dev) +static bool fimd_display_is_connected(struct exynos_drm_display *display) { /* TODO. */ return true; } -static void *fimd_get_panel(struct device *dev) +static void *fimd_get_panel(struct exynos_drm_display *display) { - struct exynos_drm_manager *mgr = get_fimd_manager(dev); - struct fimd_context *ctx = mgr->ctx; + struct fimd_context *ctx = display->ctx; return &ctx->panel; } -static int fimd_check_mode(struct device *dev, struct drm_display_mode *mode) +static int fimd_check_mode(struct exynos_drm_display *display, + struct drm_display_mode *mode) { /* TODO. */ @@ -170,12 +170,16 @@ static int fimd_check_mode(struct device *dev, struct drm_display_mode *mode) } static struct exynos_drm_display_ops fimd_display_ops = { - .type = EXYNOS_DISPLAY_TYPE_LCD, .is_connected = fimd_display_is_connected, .get_panel = fimd_get_panel, .check_mode = fimd_check_mode, }; +static struct exynos_drm_display fimd_display = { + .type = EXYNOS_DISPLAY_TYPE_LCD, + .ops = &fimd_display_ops, +}; + static void fimd_win_mode_set(struct exynos_drm_manager *mgr, struct exynos_drm_overlay *overlay) { @@ -484,15 +488,45 @@ static void fimd_win_disable(struct exynos_drm_manager *mgr, int zpos) } static int fimd_mgr_initialize(struct exynos_drm_manager *mgr, - struct drm_device *drm_dev) + struct drm_device *drm_dev, int pipe) { struct fimd_context *ctx = mgr->ctx; ctx->drm_dev = drm_dev; + ctx->pipe = pipe; + + /* + * enable drm irq mode. + * - with irq_enabled = 1, we can use the vblank feature. + * + * P.S. note that we wouldn't use drm irq handler but + * just specific driver own one instead because + * drm framework supports only one irq handler. + */ + ctx->drm_dev->irq_enabled = 1; + + /* + * with vblank_disable_allowed = 1, vblank interrupt will be disabled + * by drm timer once a current process gives up ownership of + * vblank event.(after drm_vblank_put function is called) + */ + drm_dev->vblank_disable_allowed = 1; + + /* attach this sub driver to iommu mapping if supported. */ + if (is_drm_iommu_supported(ctx->drm_dev)) + drm_iommu_attach_device(ctx->drm_dev, ctx->dev); return 0; } +static void fimd_mgr_remove(struct exynos_drm_manager *mgr) +{ + struct fimd_context *ctx = mgr->ctx; + + if (is_drm_iommu_supported(ctx->drm_dev)) + drm_iommu_detach_device(ctx->drm_dev, ctx->dev); +} + static void fimd_dpms(struct exynos_drm_manager *mgr, int mode) { struct fimd_context *ctx = mgr->ctx; @@ -526,23 +560,6 @@ static void fimd_dpms(struct exynos_drm_manager *mgr, int mode) mutex_unlock(&ctx->lock); } -static void fimd_apply(struct exynos_drm_manager *mgr) -{ - struct fimd_context *ctx = mgr->ctx; - struct exynos_drm_manager_ops *mgr_ops = mgr->ops; - struct fimd_win_data *win_data; - int i; - - for (i = 0; i < WINDOWS_NR; i++) { - win_data = &ctx->win_data[i]; - if (win_data->enabled && (mgr_ops && mgr_ops->win_commit)) - mgr_ops->win_commit(mgr, i); - } - - if (mgr_ops && mgr_ops->commit) - mgr_ops->commit(mgr); -} - static void fimd_commit(struct exynos_drm_manager *mgr) { struct fimd_context *ctx = mgr->ctx; @@ -599,6 +616,21 @@ static void fimd_commit(struct exynos_drm_manager *mgr) writel(val, ctx->regs + VIDCON0); } +static void fimd_apply(struct exynos_drm_manager *mgr) +{ + struct fimd_context *ctx = mgr->ctx; + struct fimd_win_data *win_data; + int i; + + for (i = 0; i < WINDOWS_NR; i++) { + win_data = &ctx->win_data[i]; + if (win_data->enabled) + fimd_win_commit(mgr, i); + } + + fimd_commit(mgr); +} + static int fimd_enable_vblank(struct exynos_drm_manager *mgr) { struct fimd_context *ctx = mgr->ctx; @@ -663,6 +695,7 @@ static void fimd_wait_for_vblank(struct exynos_drm_manager *mgr) static struct exynos_drm_manager_ops fimd_manager_ops = { .initialize = fimd_mgr_initialize, + .remove = fimd_mgr_remove, .dpms = fimd_dpms, .commit = fimd_commit, .enable_vblank = fimd_enable_vblank, @@ -674,16 +707,13 @@ static struct exynos_drm_manager_ops fimd_manager_ops = { }; static struct exynos_drm_manager fimd_manager = { - .pipe = -1, - .ops = &fimd_manager_ops, - .display_ops = &fimd_display_ops, + .type = EXYNOS_DISPLAY_TYPE_LCD, + .ops = &fimd_manager_ops, }; static irqreturn_t fimd_irq_handler(int irq, void *dev_id) { struct fimd_context *ctx = (struct fimd_context *)dev_id; - struct exynos_drm_subdrv *subdrv = &ctx->subdrv; - struct exynos_drm_manager *manager = subdrv->manager; u32 val; val = readl(ctx->regs + VIDINTCON1); @@ -693,11 +723,11 @@ static irqreturn_t fimd_irq_handler(int irq, void *dev_id) writel(VIDINTCON1_INT_FRAME, ctx->regs + VIDINTCON1); /* check the crtc is detached already from encoder */ - if (manager->pipe < 0 || !ctx->drm_dev) + if (ctx->pipe < 0 || !ctx->drm_dev) goto out; - drm_handle_vblank(ctx->drm_dev, manager->pipe); - exynos_drm_crtc_finish_pageflip(ctx->drm_dev, manager->pipe); + drm_handle_vblank(ctx->drm_dev, ctx->pipe); + exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe); /* set wait vsync event to zero and wake up queue. */ if (atomic_read(&ctx->wait_vsync_event)) { @@ -708,39 +738,6 @@ out: return IRQ_HANDLED; } -static int fimd_subdrv_probe(struct drm_device *drm_dev, struct device *dev) -{ - /* - * enable drm irq mode. - * - with irq_enabled = 1, we can use the vblank feature. - * - * P.S. note that we wouldn't use drm irq handler but - * just specific driver own one instead because - * drm framework supports only one irq handler. - */ - drm_dev->irq_enabled = 1; - - /* - * with vblank_disable_allowed = 1, vblank interrupt will be disabled - * by drm timer once a current process gives up ownership of - * vblank event.(after drm_vblank_put function is called) - */ - drm_dev->vblank_disable_allowed = 1; - - /* attach this sub driver to iommu mapping if supported. */ - if (is_drm_iommu_supported(drm_dev)) - drm_iommu_attach_device(drm_dev, dev); - - return 0; -} - -static void fimd_subdrv_remove(struct drm_device *drm_dev, struct device *dev) -{ - /* detach this sub driver from iommu mapping if supported. */ - if (is_drm_iommu_supported(drm_dev)) - drm_iommu_detach_device(drm_dev, dev); -} - static int fimd_configure_clocks(struct fimd_context *ctx, struct device *dev) { struct videomode *vm = &ctx->panel.vm; @@ -826,9 +823,8 @@ static int fimd_clock(struct fimd_context *ctx, bool enable) return 0; } -static void fimd_window_suspend(struct device *dev) +static void fimd_window_suspend(struct exynos_drm_manager *mgr) { - struct exynos_drm_manager *mgr = get_fimd_manager(dev); struct fimd_context *ctx = mgr->ctx; struct fimd_win_data *win_data; int i; @@ -841,9 +837,8 @@ static void fimd_window_suspend(struct device *dev) fimd_wait_for_vblank(mgr); } -static void fimd_window_resume(struct device *dev) +static void fimd_window_resume(struct exynos_drm_manager *mgr) { - struct exynos_drm_manager *mgr = get_fimd_manager(dev); struct fimd_context *ctx = mgr->ctx; struct fimd_win_data *win_data; int i; @@ -858,7 +853,6 @@ static void fimd_window_resume(struct device *dev) static int fimd_activate(struct exynos_drm_manager *mgr, bool enable) { struct fimd_context *ctx = mgr->ctx; - struct device *dev = ctx->subdrv.dev; if (enable) { int ret; @@ -873,11 +867,11 @@ static int fimd_activate(struct exynos_drm_manager *mgr, bool enable) if (test_and_clear_bit(0, &ctx->irq_flags)) fimd_enable_vblank(mgr); - fimd_window_resume(dev); + fimd_window_resume(mgr); fimd_apply(mgr); } else { - fimd_window_suspend(dev); + fimd_window_suspend(mgr); fimd_clock(ctx, false); ctx->suspended = true; @@ -914,7 +908,6 @@ static int fimd_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct fimd_context *ctx; - struct exynos_drm_subdrv *subdrv; struct resource *res; int win; int ret = -EINVAL; @@ -961,27 +954,22 @@ static int fimd_probe(struct platform_device *pdev) DRM_INIT_WAITQUEUE(&ctx->wait_vsync_queue); atomic_set(&ctx->wait_vsync_event, 0); - fimd_manager.ctx = ctx; - - subdrv = &ctx->subdrv; - - subdrv->dev = dev; - subdrv->manager = &fimd_manager; - subdrv->probe = fimd_subdrv_probe; - subdrv->remove = fimd_subdrv_remove; - mutex_init(&ctx->lock); platform_set_drvdata(pdev, &fimd_manager); + fimd_manager.ctx = ctx; + exynos_drm_manager_register(&fimd_manager); + + fimd_display.ctx = ctx; + exynos_drm_display_register(&fimd_display); + pm_runtime_enable(dev); pm_runtime_get_sync(dev); for (win = 0; win < WINDOWS_NR; win++) fimd_clear_win(ctx, win); - exynos_drm_subdrv_register(subdrv); - return 0; } @@ -991,7 +979,8 @@ static int fimd_remove(struct platform_device *pdev) struct exynos_drm_manager *mgr = platform_get_drvdata(pdev); struct fimd_context *ctx = mgr->ctx; - exynos_drm_subdrv_unregister(&ctx->subdrv); + exynos_drm_display_unregister(&fimd_display); + exynos_drm_manager_unregister(&fimd_manager); if (ctx->suspended) goto out; diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c deleted file mode 100644 index f9a9324..0000000 --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c +++ /dev/null @@ -1,449 +0,0 @@ -/* - * Copyright (C) 2011 Samsung Electronics Co.Ltd - * Authors: - * Inki Dae <inki.dae@samsung.com> - * Seung-Woo Kim <sw0312.kim@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 <drm/drmP.h> - -#include <linux/kernel.h> -#include <linux/wait.h> -#include <linux/platform_device.h> -#include <linux/pm_runtime.h> - -#include <drm/exynos_drm.h> - -#include "exynos_drm_drv.h" -#include "exynos_drm_hdmi.h" - -#define to_context(dev) platform_get_drvdata(to_platform_device(dev)) -#define to_subdrv(dev) to_context(dev) -#define get_ctx_from_subdrv(subdrv) container_of(subdrv,\ - struct drm_hdmi_context, subdrv); - -/* platform device pointer for common drm hdmi device. */ -static struct platform_device *exynos_drm_hdmi_pdev; - -/* Common hdmi subdrv needs to access the hdmi and mixer though context. -* These should be initialied by the repective drivers */ -static struct exynos_drm_hdmi_context *hdmi_ctx; -static struct exynos_drm_hdmi_context *mixer_ctx; - -/* these callback points shoud be set by specific drivers. */ -static struct exynos_hdmi_ops *hdmi_ops; -static struct exynos_mixer_ops *mixer_ops; - -struct drm_hdmi_context { - struct exynos_drm_subdrv subdrv; - struct exynos_drm_hdmi_context *hdmi_ctx; - struct exynos_drm_hdmi_context *mixer_ctx; - - bool enabled[MIXER_WIN_NR]; -}; - -int exynos_platform_device_hdmi_register(void) -{ - struct platform_device *pdev; - - if (exynos_drm_hdmi_pdev) - return -EEXIST; - - pdev = platform_device_register_simple( - "exynos-drm-hdmi", -1, NULL, 0); - if (IS_ERR(pdev)) - return PTR_ERR(pdev); - - exynos_drm_hdmi_pdev = pdev; - - return 0; -} - -void exynos_platform_device_hdmi_unregister(void) -{ - if (exynos_drm_hdmi_pdev) { - platform_device_unregister(exynos_drm_hdmi_pdev); - exynos_drm_hdmi_pdev = NULL; - } -} - -void exynos_hdmi_drv_attach(struct exynos_drm_hdmi_context *ctx) -{ - if (ctx) - hdmi_ctx = ctx; -} - -void exynos_mixer_drv_attach(struct exynos_drm_hdmi_context *ctx) -{ - if (ctx) - mixer_ctx = ctx; -} - -void exynos_hdmi_ops_register(struct exynos_hdmi_ops *ops) -{ - if (ops) - hdmi_ops = ops; -} - -void exynos_mixer_ops_register(struct exynos_mixer_ops *ops) -{ - if (ops) - mixer_ops = ops; -} - -static int drm_hdmi_display_initialize(struct device *dev, - struct drm_device *drm_dev) -{ - struct drm_hdmi_context *ctx = to_context(dev); - - if (hdmi_ops && hdmi_ops->initialize) - return hdmi_ops->initialize(ctx->hdmi_ctx->ctx, drm_dev); - - return 0; -} - - -static bool drm_hdmi_is_connected(struct device *dev) -{ - struct drm_hdmi_context *ctx = to_context(dev); - - if (hdmi_ops && hdmi_ops->is_connected) - return hdmi_ops->is_connected(ctx->hdmi_ctx->ctx); - - return false; -} - -static struct edid *drm_hdmi_get_edid(struct device *dev, - struct drm_connector *connector) -{ - struct drm_hdmi_context *ctx = to_context(dev); - - if (hdmi_ops && hdmi_ops->get_edid) - return hdmi_ops->get_edid(ctx->hdmi_ctx->ctx, connector); - - return NULL; -} -static int drm_hdmi_check_mode_ctx(struct drm_hdmi_context *ctx, - struct drm_display_mode *mode) -{ - int ret = 0; - - /* - * Both, mixer and hdmi should be able to handle the requested mode. - * If any of the two fails, return mode as BAD. - */ - - if (mixer_ops && mixer_ops->check_mode) - ret = mixer_ops->check_mode(ctx->mixer_ctx->ctx, mode); - - if (ret) - return ret; - - if (hdmi_ops && hdmi_ops->check_mode) - return hdmi_ops->check_mode(ctx->hdmi_ctx->ctx, mode); - - return 0; -} - -static int drm_hdmi_check_mode(struct device *dev, - struct drm_display_mode *mode) -{ - struct drm_hdmi_context *ctx = to_context(dev); - - return drm_hdmi_check_mode_ctx(ctx, mode); -} - -static int drm_hdmi_display_dpms(struct device *dev, int mode) -{ - struct drm_hdmi_context *ctx = to_context(dev); - - if (hdmi_ops && hdmi_ops->dpms) - hdmi_ops->dpms(ctx->hdmi_ctx->ctx, mode); - - return 0; -} - -static struct exynos_drm_display_ops drm_hdmi_display_ops = { - .type = EXYNOS_DISPLAY_TYPE_HDMI, - .initialize = drm_hdmi_display_initialize, - .is_connected = drm_hdmi_is_connected, - .get_edid = drm_hdmi_get_edid, - .check_mode = drm_hdmi_check_mode, - .dpms = drm_hdmi_display_dpms, -}; - -static int drm_hdmi_enable_vblank(struct exynos_drm_manager *mgr) -{ - struct drm_hdmi_context *ctx = mgr->ctx; - struct exynos_drm_subdrv *subdrv = &ctx->subdrv; - struct exynos_drm_manager *manager = subdrv->manager; - - if (mixer_ops && mixer_ops->enable_vblank) - return mixer_ops->enable_vblank(ctx->mixer_ctx->ctx, - manager->pipe); - - return 0; -} - -static void drm_hdmi_disable_vblank(struct exynos_drm_manager *mgr) -{ - struct drm_hdmi_context *ctx = mgr->ctx; - - if (mixer_ops && mixer_ops->disable_vblank) - return mixer_ops->disable_vblank(ctx->mixer_ctx->ctx); -} - -static void drm_hdmi_wait_for_vblank(struct exynos_drm_manager *mgr) -{ - struct drm_hdmi_context *ctx = mgr->ctx; - - if (mixer_ops && mixer_ops->wait_for_vblank) - mixer_ops->wait_for_vblank(ctx->mixer_ctx->ctx); -} - -static void drm_hdmi_mode_fixup(struct exynos_drm_manager *mgr, - struct drm_connector *connector, - const struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode) -{ - struct drm_hdmi_context *ctx = mgr->ctx; - struct drm_display_mode *m; - int mode_ok; - - drm_mode_set_crtcinfo(adjusted_mode, 0); - - mode_ok = drm_hdmi_check_mode_ctx(ctx, adjusted_mode); - - /* just return if user desired mode exists. */ - if (mode_ok == 0) - return; - - /* - * otherwise, find the most suitable mode among modes and change it - * to adjusted_mode. - */ - list_for_each_entry(m, &connector->modes, head) { - mode_ok = drm_hdmi_check_mode_ctx(ctx, m); - - if (mode_ok == 0) { - struct drm_mode_object base; - struct list_head head; - - DRM_INFO("desired mode doesn't exist so\n"); - DRM_INFO("use the most suitable mode among modes.\n"); - - DRM_DEBUG_KMS("Adjusted Mode: [%d]x[%d] [%d]Hz\n", - m->hdisplay, m->vdisplay, m->vrefresh); - - /* preserve display mode header while copying. */ - head = adjusted_mode->head; - base = adjusted_mode->base; - memcpy(adjusted_mode, m, sizeof(*m)); - adjusted_mode->head = head; - adjusted_mode->base = base; - break; - } - } -} - -static void drm_hdmi_mode_set(struct exynos_drm_manager *mgr, void *mode) -{ - struct drm_hdmi_context *ctx = mgr->ctx; - - if (hdmi_ops && hdmi_ops->mode_set) - hdmi_ops->mode_set(ctx->hdmi_ctx->ctx, mode); -} - -static void drm_hdmi_get_max_resol(struct exynos_drm_manager *mgr, - unsigned int *width, unsigned int *height) -{ - struct drm_hdmi_context *ctx = mgr->ctx; - - if (hdmi_ops && hdmi_ops->get_max_resol) - hdmi_ops->get_max_resol(ctx->hdmi_ctx->ctx, width, height); -} - -static void drm_hdmi_commit(struct exynos_drm_manager *mgr) -{ - struct drm_hdmi_context *ctx = mgr->ctx; - - if (hdmi_ops && hdmi_ops->commit) - hdmi_ops->commit(ctx->hdmi_ctx->ctx); -} - -static int drm_hdmi_mgr_initialize(struct exynos_drm_manager *mgr, struct drm_device *drm_dev) -{ - struct drm_hdmi_context *ctx = mgr->ctx; - int ret = 0; - - if (mixer_ops && mixer_ops->initialize) - ret = mixer_ops->initialize(ctx->mixer_ctx->ctx, drm_dev); - - if (mixer_ops->iommu_on) - mixer_ops->iommu_on(ctx->mixer_ctx->ctx, true); - - return ret; -} - -static void drm_hdmi_dpms(struct exynos_drm_manager *mgr, int mode) -{ - struct drm_hdmi_context *ctx = mgr->ctx; - - if (mixer_ops && mixer_ops->dpms) - mixer_ops->dpms(ctx->mixer_ctx->ctx, mode); - - if (hdmi_ops && hdmi_ops->dpms) - hdmi_ops->dpms(ctx->hdmi_ctx->ctx, mode); -} - -static void drm_mixer_win_mode_set(struct exynos_drm_manager *mgr, - struct exynos_drm_overlay *overlay) -{ - struct drm_hdmi_context *ctx = mgr->ctx; - - if (mixer_ops && mixer_ops->win_mode_set) - mixer_ops->win_mode_set(ctx->mixer_ctx->ctx, overlay); -} - -static void drm_mixer_win_commit(struct exynos_drm_manager *mgr, int zpos) -{ - struct drm_hdmi_context *ctx = mgr->ctx; - int win = (zpos == DEFAULT_ZPOS) ? MIXER_DEFAULT_WIN : zpos; - - if (win < 0 || win >= MIXER_WIN_NR) { - DRM_ERROR("mixer window[%d] is wrong\n", win); - return; - } - - if (mixer_ops && mixer_ops->win_commit) - mixer_ops->win_commit(ctx->mixer_ctx->ctx, win); - - ctx->enabled[win] = true; -} - -static void drm_mixer_win_disable(struct exynos_drm_manager *mgr, int zpos) -{ - struct drm_hdmi_context *ctx = mgr->ctx; - int win = (zpos == DEFAULT_ZPOS) ? MIXER_DEFAULT_WIN : zpos; - - if (win < 0 || win >= MIXER_WIN_NR) { - DRM_ERROR("mixer window[%d] is wrong\n", win); - return; - } - - if (mixer_ops && mixer_ops->win_disable) - mixer_ops->win_disable(ctx->mixer_ctx->ctx, win); - - ctx->enabled[win] = false; -} - -static struct exynos_drm_manager_ops drm_hdmi_manager_ops = { - .initialize = drm_hdmi_mgr_initialize, - .dpms = drm_hdmi_dpms, - .enable_vblank = drm_hdmi_enable_vblank, - .disable_vblank = drm_hdmi_disable_vblank, - .wait_for_vblank = drm_hdmi_wait_for_vblank, - .mode_fixup = drm_hdmi_mode_fixup, - .mode_set = drm_hdmi_mode_set, - .get_max_resol = drm_hdmi_get_max_resol, - .commit = drm_hdmi_commit, - .win_mode_set = drm_mixer_win_mode_set, - .win_commit = drm_mixer_win_commit, - .win_disable = drm_mixer_win_disable, -}; - -static struct exynos_drm_manager hdmi_manager = { - .pipe = -1, - .ops = &drm_hdmi_manager_ops, - .display_ops = &drm_hdmi_display_ops, -}; - -static int hdmi_subdrv_probe(struct drm_device *drm_dev, - struct device *dev) -{ - struct exynos_drm_subdrv *subdrv = to_subdrv(dev); - struct drm_hdmi_context *ctx; - - if (!hdmi_ctx) { - DRM_ERROR("hdmi context not initialized.\n"); - return -EFAULT; - } - - if (!mixer_ctx) { - DRM_ERROR("mixer context not initialized.\n"); - return -EFAULT; - } - - ctx = get_ctx_from_subdrv(subdrv); - - if (!ctx) { - DRM_ERROR("no drm hdmi context.\n"); - return -EFAULT; - } - - ctx->hdmi_ctx = hdmi_ctx; - ctx->mixer_ctx = mixer_ctx; - - return 0; -} - -static void hdmi_subdrv_remove(struct drm_device *drm_dev, struct device *dev) -{ - struct drm_hdmi_context *ctx; - struct exynos_drm_subdrv *subdrv = to_subdrv(dev); - - ctx = get_ctx_from_subdrv(subdrv); - - if (mixer_ops->iommu_on) - mixer_ops->iommu_on(ctx->mixer_ctx->ctx, false); -} - -static int exynos_drm_hdmi_probe(struct platform_device *pdev) -{ - struct device *dev = &pdev->dev; - struct exynos_drm_subdrv *subdrv; - struct drm_hdmi_context *ctx; - - ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); - if (!ctx) - return -ENOMEM; - - hdmi_manager.ctx = ctx; - - subdrv = &ctx->subdrv; - - subdrv->dev = dev; - subdrv->manager = &hdmi_manager; - subdrv->probe = hdmi_subdrv_probe; - subdrv->remove = hdmi_subdrv_remove; - - platform_set_drvdata(pdev, subdrv); - - exynos_drm_subdrv_register(subdrv); - - return 0; -} - -static int exynos_drm_hdmi_remove(struct platform_device *pdev) -{ - struct drm_hdmi_context *ctx = platform_get_drvdata(pdev); - - exynos_drm_subdrv_unregister(&ctx->subdrv); - - return 0; -} - -struct platform_driver exynos_drm_common_hdmi_driver = { - .probe = exynos_drm_hdmi_probe, - .remove = exynos_drm_hdmi_remove, - .driver = { - .name = "exynos-drm-hdmi", - .owner = THIS_MODULE, - }, -}; diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h index 923239b..37059ea 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h @@ -19,10 +19,12 @@ * exynos hdmi common context structure. * * @drm_dev: pointer to drm_device. + * @pipe: pipe for mixer * @ctx: pointer to the context of specific device driver. * this context should be hdmi_context or mixer_context. */ struct exynos_drm_hdmi_context { + int pipe; void *ctx; }; diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c index cff3aed..e0db2b3 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c @@ -13,7 +13,7 @@ #include <drm/exynos_drm.h> #include "exynos_drm_drv.h" -#include "exynos_drm_encoder.h" +#include "exynos_drm_crtc.h" #include "exynos_drm_fb.h" #include "exynos_drm_gem.h" #include "exynos_drm_plane.h" @@ -139,7 +139,7 @@ int exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc, overlay->crtc_x, overlay->crtc_y, overlay->crtc_width, overlay->crtc_height); - exynos_drm_fn_encoder(crtc, overlay, exynos_drm_encoder_plane_mode_set); + exynos_drm_crtc_plane_mode_set(crtc, overlay); return 0; } @@ -149,8 +149,7 @@ void exynos_plane_commit(struct drm_plane *plane) struct exynos_plane *exynos_plane = to_exynos_plane(plane); struct exynos_drm_overlay *overlay = &exynos_plane->overlay; - exynos_drm_fn_encoder(plane->crtc, &overlay->zpos, - exynos_drm_encoder_plane_commit); + exynos_drm_crtc_plane_commit(plane->crtc, overlay->zpos); } void exynos_plane_dpms(struct drm_plane *plane, int mode) @@ -162,17 +161,13 @@ void exynos_plane_dpms(struct drm_plane *plane, int mode) if (exynos_plane->enabled) return; - exynos_drm_fn_encoder(plane->crtc, &overlay->zpos, - exynos_drm_encoder_plane_enable); - + exynos_drm_crtc_plane_enable(plane->crtc, overlay->zpos); exynos_plane->enabled = true; } else { if (!exynos_plane->enabled) return; - exynos_drm_fn_encoder(plane->crtc, &overlay->zpos, - exynos_drm_encoder_plane_disable); - + exynos_drm_crtc_plane_disable(plane->crtc, overlay->zpos); exynos_plane->enabled = false; } }

[v2,12/26] drm/exynos: Split manager/display/subdrv

Commit Message

Comments

Patch