On Fri, 25 Nov 2016 16:49:04 +0000
Brian Starkey <brian.starkey@xxxxxxx> wrote:
> +static int
> +malidp_mw_encoder_atomic_check(struct drm_encoder *encoder,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state)
> +{
> + struct malidp_mw_connector_state *mw_state = to_mw_state(conn_state);
> + struct malidp_drm *malidp = encoder->dev->dev_private;
> + struct drm_framebuffer *fb;
> + int i, n_planes;
> +
> + if (!conn_state->writeback_job || !conn_state->writeback_job->fb)
> + return 0;
> +
> + fb = conn_state->writeback_job->fb;
> + if ((fb->width != crtc_state->mode.hdisplay) ||
> + (fb->height != crtc_state->mode.vdisplay)) {
> + DRM_DEBUG_KMS("Invalid framebuffer size %ux%u\n",
> + fb->width, fb->height);
> + return -EINVAL;
> + }
These checks look pretty generic to me. Shouldn't we have a default
helper doing that?
> +
> + mw_state->format =
> + malidp_hw_get_format_id(&malidp->dev->map, SE_MEMWRITE,
> + fb->pixel_format);
> + if (mw_state->format == MALIDP_INVALID_FORMAT_ID) {
Same goes here. By adding a format_types table similar to what is
exposed in drm_plane [1], we could do this check in the core. The only
thing left to the driver is the 4CC -> driver-specific-id conversion.
> + struct drm_format_name_buf format_name;
> +
> + DRM_DEBUG_KMS("Invalid pixel format %s\n",
> + drm_get_format_name(fb->pixel_format, &format_name));
> + return -EINVAL;
> + }
> +
> + n_planes = drm_format_num_planes(fb->pixel_format);
> + for (i = 0; i < n_planes; i++) {
> + struct drm_gem_cma_object *obj = drm_fb_cma_get_gem_obj(fb, i);
> + if (!malidp_hw_pitch_valid(malidp->dev, fb->pitches[i])) {
> + DRM_DEBUG_KMS("Invalid pitch %u for plane %d\n",
> + fb->pitches[i], i);
> + return -EINVAL;
> + }
> + mw_state->pitches[i] = fb->pitches[i];
> + mw_state->addrs[i] = obj->paddr + fb->offsets[i];
> + }
> + mw_state->n_planes = n_planes;
> +
> + return 0;
> +}
[1]http://lxr.free-electrons.com/source/include/drm/drm_plane.h#L482
