linux-kernel - Re: [PATCH 6/6] drm: mali-dp: Add writeback connector

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170414114700.552acc82@bbrezillon>
Date:   Fri, 14 Apr 2017 11:47:00 +0200
From:   Boris Brezillon <boris.brezillon@...e-electrons.com>
To:     Brian Starkey <brian.starkey@....com>
Cc:     dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        liviu.dudau@....com, laurent.pinchart@...asonboard.com,
        linux-media@...r.kernel.org
Subject: Re: [PATCH 6/6] drm: mali-dp: Add writeback connector

On Fri, 25 Nov 2016 16:49:04 +0000
Brian Starkey <brian.starkey@....com> 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