Hi Philipp,
On Wed, Dec 16, 2015 at 5:52 PM, Philipp Zabel p.zabel@pengutronix.de wrote:
Hi Daniel,
Am Dienstag, den 15.12.2015, 02:57 +0800 schrieb Daniel Kurtz:
HI Philipp,
This driver is looking really good.
But, still some things to think about (mostly small) inline below...
Most of my answers below seem to be "ok" or some form thereof, but I have one or two questions regarding the layer_config and crtc_reset suggestions.
Answers to your questions below...
[...]
+static void mtk_ovl_layer_config(void __iomem *ovl_base, unsigned int idx,
struct mtk_plane_state *state)+{
struct mtk_plane_pending_state *pending = &state->pending;unsigned int addr = pending->addr;unsigned int pitch = pending->pitch & 0xffff;unsigned int fmt = pending->format;unsigned int offset = (pending->y << 16) | pending->x;unsigned int src_size = (pending->height << 16) | pending->width;unsigned int con;con = has_rb_swapped(fmt) << 24 | ovl_fmt_convert(fmt) << 12;Call these conversion routines earlier (during atomic_check) and just add the resulting "con" value to pending.
You mean to add a .layer_atomic_check callback to the mtk_ddp_comp ops?
I didn't have any particular implementation in mind. But, yeah... maybe a new "check" callback to pre-compute and formally check the provided state. This might be overkill though if it ends up adding a lot of overhead for these values which can never really fail anyway.
[...]
How about this:
static void mtk_drm_crtc_reset(struct drm_crtc *crtc) { struct mtk_crtc_state *state;
if (crtc->state) { if (crtc->state->mode_blob) drm_property_unreference_blob(crtc->state->mode_blob); state = to_mtk_crtc_state(crtc->state); memset(state, 0, sizeof(*state)); } else { state = kzalloc(sizeof(*state), GFP_KERNEL); if (!state) return; crtc->state = &state->base; } state->base.crtc = crtc;}
lgtm
[...]
Thanks for the review!
Thanks for the patches!!
regards Philipp