From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from galahad.ideasonboard.com ([185.26.127.97]:48364 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751749AbeDDVFI (ORCPT ); Wed, 4 Apr 2018 17:05:08 -0400 From: Laurent Pinchart To: Kieran Bingham Cc: Laurent Pinchart , linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-renesas-soc@vger.kernel.org Subject: Re: [PATCH 10/15] v4l: vsp1: Move DRM pipeline output setup code to a function Date: Thu, 05 Apr 2018 00:05:17 +0300 Message-ID: <2064092.kPXj3Peec5@avalon> In-Reply-To: <4a4f5fd1-4345-5f74-2a10-dadd0e1ba130@ideasonboard.com> References: <20180226214516.11559-1-laurent.pinchart+renesas@ideasonboard.com> <3938270.yYcQyIxAEm@avalon> <4a4f5fd1-4345-5f74-2a10-dadd0e1ba130@ideasonboard.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-renesas-soc-owner@vger.kernel.org List-ID: Hi Kieran, On Wednesday, 4 April 2018 19:15:19 EEST Kieran Bingham wrote: > On 02/04/18 13:35, Laurent Pinchart wrote: > > > > >>> +/* Setup the output side of the pipeline (WPF and LIF). */ > >>> +static int vsp1_du_pipeline_setup_output(struct vsp1_device *vsp1, > >>> + struct vsp1_pipeline *pipe) > >>> +{ > >>> + struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe); > >>> + struct v4l2_subdev_format format = { > >>> + .which = V4L2_SUBDEV_FORMAT_ACTIVE, > >> > >> Why do you initialise this .which here, but all the other member > >> variables below. > >> > >> Wouldn't it make more sense to group all of this initialisation together? > >> or is there a distinction in keeping the .which separate. > >> > >> (Perhaps this is just a way to initialise the rest of the structure to 0, > >> without using the memset?) > > > > The initialization of the .which field is indeed there to avoid the > > memset, but other than that there's no particular reason. I find it > > clearer to keep the initialization of the structure close to the code that > > makes use of it (the next v4l2_subdev_call in this case). > > > > As initializing all members when declaring the variable doesn't make a > > change in code size (gcc 6.4.0) but increases .rodata by 18 bytes and > > decreases __modver by the same amount, I'm tempted to leave it as-is > > unless you think it should be changed. > > I'm happy to leave it as is - the query was as much to understand why the > change was the way it was :D > > But on that logic (reducing .rodata, or rather not increasing it) what's the > benefit of initialising with one (random/psuedo random) member variable > over initialising to all zero, then initialising the .which alongside the > rest of them? Wouldn't the compiler just use the zero page or such to > initialise then? I've just tested that, and it seems to generate the exact same code. I'll initialize the structure to 0 when declaring it and move the which field initialization with the other fields. > This way is fine if you are happy with how it reads :D > > >>> + }; > >>> + int ret; > >>> + > >>> + format.pad = RWPF_PAD_SINK; > >>> + format.format.width = drm_pipe->width; > >>> + format.format.height = drm_pipe->height; > >>> + format.format.code = MEDIA_BUS_FMT_ARGB8888_1X32; > >>> + format.format.field = V4L2_FIELD_NONE; > >>> + > >>> + ret = v4l2_subdev_call(&pipe->output->entity.subdev, pad, set_fmt, > > > > NULL, > > > >>> + &format); > >>> + if (ret < 0) > >>> + return ret; > >>> + -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH 10/15] v4l: vsp1: Move DRM pipeline output setup code to a function Date: Thu, 05 Apr 2018 00:05:17 +0300 Message-ID: <2064092.kPXj3Peec5@avalon> References: <20180226214516.11559-1-laurent.pinchart+renesas@ideasonboard.com> <3938270.yYcQyIxAEm@avalon> <4a4f5fd1-4345-5f74-2a10-dadd0e1ba130@ideasonboard.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from galahad.ideasonboard.com (galahad.ideasonboard.com [185.26.127.97]) by gabe.freedesktop.org (Postfix) with ESMTPS id 82FDA6E62B for ; Wed, 4 Apr 2018 21:05:08 +0000 (UTC) In-Reply-To: <4a4f5fd1-4345-5f74-2a10-dadd0e1ba130@ideasonboard.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Kieran Bingham Cc: linux-renesas-soc@vger.kernel.org, Laurent Pinchart , dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org SGkgS2llcmFuLAoKT24gV2VkbmVzZGF5LCA0IEFwcmlsIDIwMTggMTk6MTU6MTkgRUVTVCBLaWVy YW4gQmluZ2hhbSB3cm90ZToKPiBPbiAwMi8wNC8xOCAxMzozNSwgTGF1cmVudCBQaW5jaGFydCB3 cm90ZToKPiAKPiA8c25pcD4KPiAKPiA+Pj4gKy8qIFNldHVwIHRoZSBvdXRwdXQgc2lkZSBvZiB0 aGUgcGlwZWxpbmUgKFdQRiBhbmQgTElGKS4gKi8KPiA+Pj4gK3N0YXRpYyBpbnQgdnNwMV9kdV9w aXBlbGluZV9zZXR1cF9vdXRwdXQoc3RydWN0IHZzcDFfZGV2aWNlICp2c3AxLAo+ID4+PiArCQkJ CQkgc3RydWN0IHZzcDFfcGlwZWxpbmUgKnBpcGUpCj4gPj4+ICt7Cj4gPj4+ICsJc3RydWN0IHZz cDFfZHJtX3BpcGVsaW5lICpkcm1fcGlwZSA9IHRvX3ZzcDFfZHJtX3BpcGVsaW5lKHBpcGUpOwo+ ID4+PiArCXN0cnVjdCB2NGwyX3N1YmRldl9mb3JtYXQgZm9ybWF0ID0gewo+ID4+PiArCQkud2hp Y2ggPSBWNEwyX1NVQkRFVl9GT1JNQVRfQUNUSVZFLAo+ID4+IAo+ID4+IFdoeSBkbyB5b3UgaW5p dGlhbGlzZSB0aGlzIC53aGljaCBoZXJlLCBidXQgYWxsIHRoZSBvdGhlciBtZW1iZXIKPiA+PiB2 YXJpYWJsZXMgYmVsb3cuCj4gPj4gCj4gPj4gV291bGRuJ3QgaXQgbWFrZSBtb3JlIHNlbnNlIHRv IGdyb3VwIGFsbCBvZiB0aGlzIGluaXRpYWxpc2F0aW9uIHRvZ2V0aGVyPwo+ID4+IG9yIGlzIHRo ZXJlIGEgZGlzdGluY3Rpb24gaW4ga2VlcGluZyB0aGUgLndoaWNoIHNlcGFyYXRlLgo+ID4+IAo+ ID4+IChQZXJoYXBzIHRoaXMgaXMganVzdCBhIHdheSB0byBpbml0aWFsaXNlIHRoZSByZXN0IG9m IHRoZSBzdHJ1Y3R1cmUgdG8gMCwKPiA+PiB3aXRob3V0IHVzaW5nIHRoZSBtZW1zZXQ/KQo+ID4g Cj4gPiBUaGUgaW5pdGlhbGl6YXRpb24gb2YgdGhlIC53aGljaCBmaWVsZCBpcyBpbmRlZWQgdGhl cmUgdG8gYXZvaWQgdGhlCj4gPiBtZW1zZXQsIGJ1dCBvdGhlciB0aGFuIHRoYXQgdGhlcmUncyBu byBwYXJ0aWN1bGFyIHJlYXNvbi4gSSBmaW5kIGl0Cj4gPiBjbGVhcmVyIHRvIGtlZXAgdGhlIGlu aXRpYWxpemF0aW9uIG9mIHRoZSBzdHJ1Y3R1cmUgY2xvc2UgdG8gdGhlIGNvZGUgdGhhdAo+ID4g bWFrZXMgdXNlIG9mIGl0ICh0aGUgbmV4dCB2NGwyX3N1YmRldl9jYWxsIGluIHRoaXMgY2FzZSku Cj4gPiAKPiA+IEFzIGluaXRpYWxpemluZyBhbGwgbWVtYmVycyB3aGVuIGRlY2xhcmluZyB0aGUg dmFyaWFibGUgZG9lc24ndCBtYWtlIGEKPiA+IGNoYW5nZSBpbiBjb2RlIHNpemUgKGdjYyA2LjQu MCkgYnV0IGluY3JlYXNlcyAucm9kYXRhIGJ5IDE4IGJ5dGVzIGFuZAo+ID4gZGVjcmVhc2VzIF9f bW9kdmVyIGJ5IHRoZSBzYW1lIGFtb3VudCwgSSdtIHRlbXB0ZWQgdG8gbGVhdmUgaXQgYXMtaXMK PiA+IHVubGVzcyB5b3UgdGhpbmsgaXQgc2hvdWxkIGJlIGNoYW5nZWQuCj4gCj4gSSdtIGhhcHB5 IHRvIGxlYXZlIGl0IGFzIGlzIC0gdGhlIHF1ZXJ5IHdhcyBhcyBtdWNoIHRvIHVuZGVyc3RhbmQg d2h5IHRoZQo+IGNoYW5nZSB3YXMgdGhlIHdheSBpdCB3YXMgOkQKPiAKPiBCdXQgb24gdGhhdCBs b2dpYyAocmVkdWNpbmcgLnJvZGF0YSwgb3IgcmF0aGVyIG5vdCBpbmNyZWFzaW5nIGl0KSB3aGF0 J3MgdGhlCj4gYmVuZWZpdCBvZiBpbml0aWFsaXNpbmcgd2l0aCBvbmUgKHJhbmRvbS9wc3VlZG8g cmFuZG9tKSBtZW1iZXIgdmFyaWFibGUKPiBvdmVyIGluaXRpYWxpc2luZyB0byBhbGwgemVybywg dGhlbiBpbml0aWFsaXNpbmcgdGhlIC53aGljaCBhbG9uZ3NpZGUgdGhlCj4gcmVzdCBvZiB0aGVt PyBXb3VsZG4ndCB0aGUgY29tcGlsZXIganVzdCB1c2UgdGhlIHplcm8gcGFnZSBvciBzdWNoIHRv Cj4gaW5pdGlhbGlzZSB0aGVuPwoKSSd2ZSBqdXN0IHRlc3RlZCB0aGF0LCBhbmQgaXQgc2VlbXMg dG8gZ2VuZXJhdGUgdGhlIGV4YWN0IHNhbWUgY29kZS4gSSdsbCAKaW5pdGlhbGl6ZSB0aGUgc3Ry dWN0dXJlIHRvIDAgd2hlbiBkZWNsYXJpbmcgaXQgYW5kIG1vdmUgdGhlIHdoaWNoIGZpZWxkIApp bml0aWFsaXphdGlvbiB3aXRoIHRoZSBvdGhlciBmaWVsZHMuCgo+IFRoaXMgd2F5IGlzIGZpbmUg aWYgeW91IGFyZSBoYXBweSB3aXRoIGhvdyBpdCByZWFkcyA6RAo+IAo+ID4+PiArCX07Cj4gPj4+ ICsJaW50IHJldDsKPiA+Pj4gKwo+ID4+PiArCWZvcm1hdC5wYWQgPSBSV1BGX1BBRF9TSU5LOwo+ ID4+PiArCWZvcm1hdC5mb3JtYXQud2lkdGggPSBkcm1fcGlwZS0+d2lkdGg7Cj4gPj4+ICsJZm9y bWF0LmZvcm1hdC5oZWlnaHQgPSBkcm1fcGlwZS0+aGVpZ2h0Owo+ID4+PiArCWZvcm1hdC5mb3Jt YXQuY29kZSA9IE1FRElBX0JVU19GTVRfQVJHQjg4ODhfMVgzMjsKPiA+Pj4gKwlmb3JtYXQuZm9y bWF0LmZpZWxkID0gVjRMMl9GSUVMRF9OT05FOwo+ID4+PiArCj4gPj4+ICsJcmV0ID0gdjRsMl9z dWJkZXZfY2FsbCgmcGlwZS0+b3V0cHV0LT5lbnRpdHkuc3ViZGV2LCBwYWQsIHNldF9mbXQsCj4g PiAKPiA+IE5VTEwsCj4gPiAKPiA+Pj4gKwkJCSAgICAgICAmZm9ybWF0KTsKPiA+Pj4gKwlpZiAo cmV0IDwgMCkKPiA+Pj4gKwkJcmV0dXJuIHJldDsKPiA+Pj4gKwoKLS0gClJlZ2FyZHMsCgpMYXVy ZW50IFBpbmNoYXJ0CgoKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9w Lm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1k ZXZlbAo=