Sean Paul has posted comments on this change.
Change subject: drm/exynos: Refactor manager/display/overlay
......................................................................
Patch Set 2: (6 inline comments)
Thanks for the review, some responses inline. I'll respin.
....................................................
File drivers/gpu/drm/exynos/exynos_drm_connector.c
Line 294: #ifdef CONFIG_DRM_EXYNOS_HDMI
They're not. It's kruft that I added early on so I didn't have to deal with allocated displays that do not have implementations.
Will remove.
....................................................
File drivers/gpu/drm/exynos/exynos_drm_display.c
Line 25:
No, I originally thought we might be able to move common code in here, but there's not really enough to justify it.
I'll remove.
Line 183:
Yeah. This is a little messed up, and relates to one of my TODOs elsewhere in the code.
I originally had dpms pointing to power, but realized that we're calling dpms, then power one after another on init. Since no one implements both, we're fine.
Like other places in the code, dpms/power needs to be cleaned up.
....................................................
File drivers/gpu/drm/exynos/exynos_drm_display.h
Line 65: struct exynos_controller_ops {
Unfortunately, this doesn't map to encoder_ops; some, like page_flip, are crtc ops.
These are meant to map directly to functions of FIMD/Mixer/VIDI, not drm constructs.
I'm happy to rename it, but I fear encoder_ops might make things more confusing.
....................................................
File drivers/gpu/drm/exynos/exynos_drm_encoder.c
Line 201: #ifdef CONFIG_DRM_EXYNOS_FIMD
Will remove.
Line 317: * enable_vblank hasn't already been called, that's a bug.
Yeah, your theory makes sense to me. I still think it's a hack, and needs more investigation. I plan on coming back to fix up all of the TODOs, but I wanted to avoid increasing the scope of this (already large) CL.
--
To view, visit https://gerrit.chromium.org/gerrit/36477
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1bd6bbd67d49af2abecf36c43bfe765ae00f38d
Gerrit-PatchSet: 2
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.4
Gerrit-Owner: Sean Paul <seanp...@chromium.org>
Gerrit-Reviewer: Gerrit <chrome-...@google.com>
Gerrit-Reviewer: Sean Paul <seanp...@chromium.org>
Gerrit-Reviewer: Stéphane Marchesin <marc...@chromium.org>
