Hi Philipp,
On Wed, Dec 16, 2015 at 5:52 PM, Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> 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
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
