| Message ID | 1490706496-4959-3-git-send-email-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Tomi, Thank you for the patch. On Tuesday 28 Mar 2017 16:07:48 Tomi Valkeinen wrote: > The current driver doesn't expose any of the CRTC HW properties like > background color or transparency key, and sets them at CRTC enable time. > > Refactor this into a separate function and call that function from > omap_crtc_atomic_flush(). This is the behavior we want when the > properties can be configured, so this patch makes it easier to add > patches later which implement those properties. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/omapdrm/omap_crtc.c | 25 +++++++++++++++++-------- > 1 file changed, 17 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c > b/drivers/gpu/drm/omapdrm/omap_crtc.c index 2fe735c269fc..49fc61963af4 > 100644 > --- a/drivers/gpu/drm/omapdrm/omap_crtc.c > +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c > @@ -198,15 +198,7 @@ static void omap_crtc_set_enabled(struct drm_crtc > *crtc, bool enable) static int omap_crtc_dss_enable(enum omap_channel > channel) > { > struct omap_crtc *omap_crtc = omap_crtcs[channel]; > - struct omap_overlay_manager_info info; > > - memset(&info, 0, sizeof(info)); > - info.default_color = 0x00000000; > - info.trans_key = 0x00000000; > - info.trans_key_type = OMAP_DSS_COLOR_KEY_GFX_DST; > - info.trans_enabled = false; > - > - dispc_mgr_setup(omap_crtc->channel, &info); > dispc_mgr_set_timings(omap_crtc->channel, > &omap_crtc->vm); > omap_crtc_set_enabled(&omap_crtc->base, true); > @@ -313,6 +305,21 @@ void omap_crtc_vblank_irq(struct drm_crtc *crtc) > DBG("%s: apply done", omap_crtc->name); > } > > +static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc) > +{ > + struct omap_crtc *omap_crtc = to_omap_crtc(crtc); > + struct omap_overlay_manager_info info; > + > + memset(&info, 0, sizeof(info)); > + > + info.default_color = 0x000000; > + info.trans_enabled = false; > + info.partial_alpha_enabled = false; > + info.cpr_enable = false; Nitpicking, you could initialize info when declaring it, I think it would be slightly more efficient. > + > + dispc_mgr_setup(omap_crtc->channel, &info); dispc_mgr_setup() was previously called from omap_crtc_dss_enable() with the DSS disabled. Now it can be called with the DSS enabled. Have you double- checked that this isn't an issue ? > +} > + > /* ------------------------------------------------------------------------ > * CRTC Functions > */ > @@ -410,6 +417,8 @@ static void omap_crtc_atomic_flush(struct drm_crtc > *crtc, dispc_mgr_set_gamma(omap_crtc->channel, lut, length); > } > > + omap_crtc_write_crtc_properties(crtc); > + > /* Only flush the CRTC if it is currently enabled. */ > if (!omap_crtc->enabled) > return;
On 29/03/17 11:05, Laurent Pinchart wrote: >> +static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc) >> +{ >> + struct omap_crtc *omap_crtc = to_omap_crtc(crtc); >> + struct omap_overlay_manager_info info; >> + >> + memset(&info, 0, sizeof(info)); >> + >> + info.default_color = 0x000000; >> + info.trans_enabled = false; >> + info.partial_alpha_enabled = false; >> + info.cpr_enable = false; > > Nitpicking, you could initialize info when declaring it, I think it would be > slightly more efficient. These are open coded here so that it's easy to change these line by line when we add the properties, and that code most likely can't be in the initializer. But yes, at the moment it looks a bit silly. >> + >> + dispc_mgr_setup(omap_crtc->channel, &info); > > dispc_mgr_setup() was previously called from omap_crtc_dss_enable() with the > DSS disabled. Now it can be called with the DSS enabled. Have you double- > checked that this isn't an issue ? It's not an issue, it's how it's supposed to work. These mgr-settings are "shadow" settings, i.e. they take effect when we set the GO bit (like the plane settings). They were only written in dss_enable() as we didn't have support to change those properties. I hope will get those properties some time soon, so we need to write the mgr-settings even when the output is enabled. Tomi
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index 2fe735c269fc..49fc61963af4 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -198,15 +198,7 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable) static int omap_crtc_dss_enable(enum omap_channel channel) { struct omap_crtc *omap_crtc = omap_crtcs[channel]; - struct omap_overlay_manager_info info; - memset(&info, 0, sizeof(info)); - info.default_color = 0x00000000; - info.trans_key = 0x00000000; - info.trans_key_type = OMAP_DSS_COLOR_KEY_GFX_DST; - info.trans_enabled = false; - - dispc_mgr_setup(omap_crtc->channel, &info); dispc_mgr_set_timings(omap_crtc->channel, &omap_crtc->vm); omap_crtc_set_enabled(&omap_crtc->base, true); @@ -313,6 +305,21 @@ void omap_crtc_vblank_irq(struct drm_crtc *crtc) DBG("%s: apply done", omap_crtc->name); } +static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc) +{ + struct omap_crtc *omap_crtc = to_omap_crtc(crtc); + struct omap_overlay_manager_info info; + + memset(&info, 0, sizeof(info)); + + info.default_color = 0x000000; + info.trans_enabled = false; + info.partial_alpha_enabled = false; + info.cpr_enable = false; + + dispc_mgr_setup(omap_crtc->channel, &info); +} + /* ----------------------------------------------------------------------------- * CRTC Functions */ @@ -410,6 +417,8 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc, dispc_mgr_set_gamma(omap_crtc->channel, lut, length); } + omap_crtc_write_crtc_properties(crtc); + /* Only flush the CRTC if it is currently enabled. */ if (!omap_crtc->enabled) return;
The current driver doesn't expose any of the CRTC HW properties like background color or transparency key, and sets them at CRTC enable time. Refactor this into a separate function and call that function from omap_crtc_atomic_flush(). This is the behavior we want when the properties can be configured, so this patch makes it easier to add patches later which implement those properties. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/gpu/drm/omapdrm/omap_crtc.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)