From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] adv7511: Added mode_fixup function. Date: Tue, 9 Feb 2016 10:24:44 +0100 Message-ID: <20160209092444.GD11240@phenom.ffwll.local> References: <1454063627-12219-1-git-send-email-palminha@synopsys.com> <1948106.WC7MId72vp@avalon> <56AF5176.80603@synopsys.com> <56B36CB1.80000@synopsys.com> <56B3B42C.7020804@metafoo.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-wm0-f44.google.com (mail-wm0-f44.google.com [74.125.82.44]) by gabe.freedesktop.org (Postfix) with ESMTPS id 749E36E51E for ; Tue, 9 Feb 2016 01:24:22 -0800 (PST) Received: by mail-wm0-f44.google.com with SMTP id g62so166098125wme.0 for ; Tue, 09 Feb 2016 01:24:22 -0800 (PST) Content-Disposition: inline In-Reply-To: <56B3B42C.7020804@metafoo.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Lars-Peter Clausen Cc: Laurent Pinchart , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Carlos Palminha , Laurent Pinchart List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCBGZWIgMDQsIDIwMTYgYXQgMDk6Mjc6MjRQTSArMDEwMCwgTGFycy1QZXRlciBDbGF1 c2VuIHdyb3RlOgo+IE9uIDAyLzA0LzIwMTYgMDQ6MjIgUE0sIENhcmxvcyBQYWxtaW5oYSB3cm90 ZToKPiA+IEhpIGd1eXMsCj4gPiAKPiA+IGFueSBmZWVkYmFjaz8gcGF0Y2ggd2lsbCBiZSBhY2Nl cHRlZCBmb3IgYWR2NzUxMSBkcml2ZXI/Cj4gCj4gSGksCj4gCj4gVGhhbmtzIGZvciB0aGUgcGF0 Y2gsIGJ1dCBwbGVhc2UgdHJ5IHRvIGZpbmQgYW5kIGZpeCB0aGUgY2FsbCBzaXRlIHRoYXQgaXMK PiB0cnlpbmcgdG8gaW52b2tlIHRoZSBjYWxsYmFjayBldmVuIHRob3VnaCBpdCBkb2VzIG5vdCBl eGlzdC4KPiAKPiBUaGlzIGlzIG1vc3QgbGlrZWx5IGRybV9pMmNfZW5jb2Rlcl9tb2RlX2ZpeHVw KCkuCgpBZ3JlZWQsIHRoaXMgc2hvdWxkIGJlIGZpeGVkIGluIHRoZSBoZWxwZXIgbGlicmFyeSwg bm90IGluIGRyaXZlcnMgYnkKY29weXBhc3RpbmcgcGlsZXMgbW9yZSBkdW1teSBmdW5jdGlvbnMu Ci1EYW5pZWwKCj4gCj4gLSBMYXJzCj4gCj4gPiAKPiA+IFJlZ2FyZHMsCj4gPiBDLlBhbG1pbmhh Cj4gPiAKPiA+IE9uIDAxLTAyLTIwMTYgMTI6MzcsIENhcmxvcyBQYWxtaW5oYSB3cm90ZToKPiA+ PiBIaSBMYXVyZW50Cj4gPj4KPiA+PiBPbiAyOS0wMS0yMDE2IDE3OjQ4LCBMYXVyZW50IFBpbmNo YXJ0IHdyb3RlOgo+ID4+PiBIaSBDYXJsb3MsCj4gPj4+Cj4gPj4+IFRoYW5rIHlvdSBmb3IgdGhl IHBhdGNoLgo+ID4+Pgo+ID4+PiBPbiBGcmlkYXkgMjkgSmFudWFyeSAyMDE2IDEwOjMzOjQ3IENh cmxvcyBQYWxtaW5oYSB3cm90ZToKPiA+Pj4+IFRoZSBtb2RlX2ZpeHVwIGlzIG5lY2Vzc2FyeSB3 aGVuIHVzaW5nIGl0IGluIGEgRFJNIEZCIGRyaXZlciBwaXBlbGluZS4KPiA+Pj4KPiA+Pj4gSW5z dGVhZCBvZiBpbXBsZW1lbnRpbmcgc3R1YnMgaW4gZW5jb2RlciBkcml2ZXJzLCB3b3VsZG4ndCBp dCBiZSBiZXR0ZXIgdG8gCj4gPj4+IG1ha2UgbW9kZV9maXh1cCBvcHRpb25hbCA/Cj4gPj4gUHJv YmFibHkgeW91IGFyZSByaWdodCBidXQgaSBkb24ndCBoYXZlIGVub3VnaCBrbm93bGVkZ2Ugb3Ig dGltZSB0byBkbyB0aGF0IGZvciB0aGUgRFJNIGZyYW1ld29yay4gOigKPiA+PiBJIGxpbWl0ZWQg bXlzZWxmIHRvIGRvIHdoYXQgdGhlIG90aGVyIGRyaXZlcnMgYWxyZWFkeSBpbXBsZW1lbnQuCj4g Pj4KPiA+PiBUaGUgcGF0Y2ggaXMgbWFuZGF0b3J5IHRvIGhhdmUgdG8gdGhlIEFEViB3b3JraW5n IG9yIGVsc2Ugd2lsbCBnZXQgc29tZSBOVUxMIHBvaW50ZXIgY3Jhc2guCj4gPj4KPiA+PiBSZWdh cmRzLAo+ID4+IEMuUGFsbWluaGEKPiA+Pgo+ID4+Pgo+ID4+Pj4gU2lnbmVkLW9mZi1ieTogQ2Fy bG9zIFBhbG1pbmhhIDxwYWxtaW5oYUBzeW5vcHN5cy5jb20+Cj4gPj4+PiAtLS0KPiA+Pj4+ICBk cml2ZXJzL2dwdS9kcm0vaTJjL2Fkdjc1MTEuYyB8IDggKysrKysrKysKPiA+Pj4+ICAxIGZpbGUg Y2hhbmdlZCwgOCBpbnNlcnRpb25zKCspCj4gPj4+Pgo+ID4+Pj4gZGlmZiAtLWdpdCBhL2RyaXZl cnMvZ3B1L2RybS9pMmMvYWR2NzUxMS5jIGIvZHJpdmVycy9ncHUvZHJtL2kyYy9hZHY3NTExLmMK PiA+Pj4+IGluZGV4IDUzM2QxZTMuLjkwMDgyZDIgMTAwNjQ0Cj4gPj4+PiAtLS0gYS9kcml2ZXJz L2dwdS9kcm0vaTJjL2Fkdjc1MTEuYwo+ID4+Pj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2kyYy9h ZHY3NTExLmMKPiA+Pj4+IEBAIC02NDgsNiArNjQ4LDEzIEBAIGFkdjc1MTFfZW5jb2Rlcl9kZXRl Y3Qoc3RydWN0IGRybV9lbmNvZGVyICplbmNvZGVyLAo+ID4+Pj4gIAlyZXR1cm4gc3RhdHVzOwo+ ID4+Pj4gIH0KPiA+Pj4+Cj4gPj4+PiArc3RhdGljIGJvb2wgYWR2NzUxMV9lbmNvZGVyX21vZGVf Zml4dXAoc3RydWN0IGRybV9lbmNvZGVyICplbmNvZGVyLAo+ID4+Pj4gKwkJCQkJY29uc3Qgc3Ry dWN0IGRybV9kaXNwbGF5X21vZGUgKm1vZGUsCj4gPj4+PiArCQkJCQlzdHJ1Y3QgZHJtX2Rpc3Bs YXlfbW9kZSAqYWRqdXN0ZWRfbW9kZSkKPiA+Pj4+ICt7Cj4gPj4+PiArCXJldHVybiB0cnVlOwo+ ID4+Pj4gK30KPiA+Pj4+ICsKPiA+Pj4+ICBzdGF0aWMgaW50IGFkdjc1MTFfZW5jb2Rlcl9tb2Rl X3ZhbGlkKHN0cnVjdCBkcm1fZW5jb2RlciAqZW5jb2RlciwKPiA+Pj4+ICAJCQkJICAgICAgc3Ry dWN0IGRybV9kaXNwbGF5X21vZGUgKm1vZGUpCj4gPj4+PiAgewo+ID4+Pj4gQEAgLTc1NCw2ICs3 NjEsNyBAQCBzdGF0aWMgdm9pZCBhZHY3NTExX2VuY29kZXJfbW9kZV9zZXQoc3RydWN0IGRybV9l bmNvZGVyCj4gPj4+PiAqZW5jb2RlciwKPiA+Pj4+Cj4gPj4+PiAgc3RhdGljIGNvbnN0IHN0cnVj dCBkcm1fZW5jb2Rlcl9zbGF2ZV9mdW5jcyBhZHY3NTExX2VuY29kZXJfZnVuY3MgPSB7Cj4gPj4+ PiAgCS5kcG1zID0gYWR2NzUxMV9lbmNvZGVyX2RwbXMsCj4gPj4+PiArCS5tb2RlX2ZpeHVwID0g YWR2NzUxMV9lbmNvZGVyX21vZGVfZml4dXAsCj4gPj4+PiAgCS5tb2RlX3ZhbGlkID0gYWR2NzUx MV9lbmNvZGVyX21vZGVfdmFsaWQsCj4gPj4+PiAgCS5tb2RlX3NldCA9IGFkdjc1MTFfZW5jb2Rl cl9tb2RlX3NldCwKPiA+Pj4+ICAJLmRldGVjdCA9IGFkdjc1MTFfZW5jb2Rlcl9kZXRlY3QsCj4g Pj4+Cj4gPj4KPiA+Pgo+ID4+IE9uIDI5LTAxLTIwMTYgMTc6NDgsIExhdXJlbnQgUGluY2hhcnQg d3JvdGU6Cj4gPj4+IEhpIENhcmxvcywKPiA+Pj4KPiA+Pj4gVGhhbmsgeW91IGZvciB0aGUgcGF0 Y2guCj4gPj4+Cj4gPj4+IE9uIEZyaWRheSAyOSBKYW51YXJ5IDIwMTYgMTA6MzM6NDcgQ2FybG9z IFBhbG1pbmhhIHdyb3RlOgo+ID4+Pj4gVGhlIG1vZGVfZml4dXAgaXMgbmVjZXNzYXJ5IHdoZW4g dXNpbmcgaXQgaW4gYSBEUk0gRkIgZHJpdmVyIHBpcGVsaW5lLgo+ID4+Pgo+ID4+PiBJbnN0ZWFk IG9mIGltcGxlbWVudGluZyBzdHVicyBpbiBlbmNvZGVyIGRyaXZlcnMsIHdvdWxkbid0IGl0IGJl IGJldHRlciB0byAKPiA+Pj4gbWFrZSBtb2RlX2ZpeHVwIG9wdGlvbmFsID8KPiA+Pj4KPiA+Pj4+ IFNpZ25lZC1vZmYtYnk6IENhcmxvcyBQYWxtaW5oYSA8cGFsbWluaGFAc3lub3BzeXMuY29tPgo+ ID4+Pj4gLS0tCj4gPj4+PiAgZHJpdmVycy9ncHUvZHJtL2kyYy9hZHY3NTExLmMgfCA4ICsrKysr KysrCj4gPj4+PiAgMSBmaWxlIGNoYW5nZWQsIDggaW5zZXJ0aW9ucygrKQo+ID4+Pj4KPiA+Pj4+ IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vaTJjL2Fkdjc1MTEuYyBiL2RyaXZlcnMvZ3B1 L2RybS9pMmMvYWR2NzUxMS5jCj4gPj4+PiBpbmRleCA1MzNkMWUzLi45MDA4MmQyIDEwMDY0NAo+ ID4+Pj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2kyYy9hZHY3NTExLmMKPiA+Pj4+ICsrKyBiL2Ry aXZlcnMvZ3B1L2RybS9pMmMvYWR2NzUxMS5jCj4gPj4+PiBAQCAtNjQ4LDYgKzY0OCwxMyBAQCBh ZHY3NTExX2VuY29kZXJfZGV0ZWN0KHN0cnVjdCBkcm1fZW5jb2RlciAqZW5jb2RlciwKPiA+Pj4+ ICAJcmV0dXJuIHN0YXR1czsKPiA+Pj4+ICB9Cj4gPj4+Pgo+ID4+Pj4gK3N0YXRpYyBib29sIGFk djc1MTFfZW5jb2Rlcl9tb2RlX2ZpeHVwKHN0cnVjdCBkcm1fZW5jb2RlciAqZW5jb2RlciwKPiA+ Pj4+ICsJCQkJCWNvbnN0IHN0cnVjdCBkcm1fZGlzcGxheV9tb2RlICptb2RlLAo+ID4+Pj4gKwkJ CQkJc3RydWN0IGRybV9kaXNwbGF5X21vZGUgKmFkanVzdGVkX21vZGUpCj4gPj4+PiArewo+ID4+ Pj4gKwlyZXR1cm4gdHJ1ZTsKPiA+Pj4+ICt9Cj4gPj4+PiArCj4gPj4+PiAgc3RhdGljIGludCBh ZHY3NTExX2VuY29kZXJfbW9kZV92YWxpZChzdHJ1Y3QgZHJtX2VuY29kZXIgKmVuY29kZXIsCj4g Pj4+PiAgCQkJCSAgICAgIHN0cnVjdCBkcm1fZGlzcGxheV9tb2RlICptb2RlKQo+ID4+Pj4gIHsK PiA+Pj4+IEBAIC03NTQsNiArNzYxLDcgQEAgc3RhdGljIHZvaWQgYWR2NzUxMV9lbmNvZGVyX21v ZGVfc2V0KHN0cnVjdCBkcm1fZW5jb2Rlcgo+ID4+Pj4gKmVuY29kZXIsCj4gPj4+Pgo+ID4+Pj4g IHN0YXRpYyBjb25zdCBzdHJ1Y3QgZHJtX2VuY29kZXJfc2xhdmVfZnVuY3MgYWR2NzUxMV9lbmNv ZGVyX2Z1bmNzID0gewo+ID4+Pj4gIAkuZHBtcyA9IGFkdjc1MTFfZW5jb2Rlcl9kcG1zLAo+ID4+ Pj4gKwkubW9kZV9maXh1cCA9IGFkdjc1MTFfZW5jb2Rlcl9tb2RlX2ZpeHVwLAo+ID4+Pj4gIAku bW9kZV92YWxpZCA9IGFkdjc1MTFfZW5jb2Rlcl9tb2RlX3ZhbGlkLAo+ID4+Pj4gIAkubW9kZV9z ZXQgPSBhZHY3NTExX2VuY29kZXJfbW9kZV9zZXQsCj4gPj4+PiAgCS5kZXRlY3QgPSBhZHY3NTEx X2VuY29kZXJfZGV0ZWN0LAo+ID4+Pgo+ID4gX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX18KPiA+IGRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKPiA+IGRyaS1kZXZl bEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKPiA+IGh0dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcv bWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwKPiA+IAo+IAo+IF9fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fCj4gZHJpLWRldmVsIG1haWxpbmcgbGlzdAo+IGRy aS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKPiBodHRwOi8vbGlzdHMuZnJlZWRlc2t0b3Au b3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCgotLSAKRGFuaWVsIFZldHRlcgpTb2Z0d2Fy ZSBFbmdpbmVlciwgSW50ZWwgQ29ycG9yYXRpb24KaHR0cDovL2Jsb2cuZmZ3bGwuY2gKX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxp bmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJl ZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756051AbcBIJYZ (ORCPT ); Tue, 9 Feb 2016 04:24:25 -0500 Received: from mail-wm0-f43.google.com ([74.125.82.43]:34474 "EHLO mail-wm0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751734AbcBIJYW (ORCPT ); Tue, 9 Feb 2016 04:24:22 -0500 Date: Tue, 9 Feb 2016 10:24:44 +0100 From: Daniel Vetter To: Lars-Peter Clausen Cc: Carlos Palminha , Laurent Pinchart , David Airlie , Laurent Pinchart , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH] adv7511: Added mode_fixup function. Message-ID: <20160209092444.GD11240@phenom.ffwll.local> Mail-Followup-To: Lars-Peter Clausen , Carlos Palminha , Laurent Pinchart , David Airlie , Laurent Pinchart , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org References: <1454063627-12219-1-git-send-email-palminha@synopsys.com> <1948106.WC7MId72vp@avalon> <56AF5176.80603@synopsys.com> <56B36CB1.80000@synopsys.com> <56B3B42C.7020804@metafoo.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56B3B42C.7020804@metafoo.de> X-Operating-System: Linux phenom 4.3.0-1-amd64 User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Feb 04, 2016 at 09:27:24PM +0100, Lars-Peter Clausen wrote: > On 02/04/2016 04:22 PM, Carlos Palminha wrote: > > Hi guys, > > > > any feedback? patch will be accepted for adv7511 driver? > > Hi, > > Thanks for the patch, but please try to find and fix the call site that is > trying to invoke the callback even though it does not exist. > > This is most likely drm_i2c_encoder_mode_fixup(). Agreed, this should be fixed in the helper library, not in drivers by copypasting piles more dummy functions. -Daniel > > - Lars > > > > > Regards, > > C.Palminha > > > > On 01-02-2016 12:37, Carlos Palminha wrote: > >> Hi Laurent > >> > >> On 29-01-2016 17:48, Laurent Pinchart wrote: > >>> Hi Carlos, > >>> > >>> Thank you for the patch. > >>> > >>> On Friday 29 January 2016 10:33:47 Carlos Palminha wrote: > >>>> The mode_fixup is necessary when using it in a DRM FB driver pipeline. > >>> > >>> Instead of implementing stubs in encoder drivers, wouldn't it be better to > >>> make mode_fixup optional ? > >> Probably you are right but i don't have enough knowledge or time to do that for the DRM framework. :( > >> I limited myself to do what the other drivers already implement. > >> > >> The patch is mandatory to have to the ADV working or else will get some NULL pointer crash. > >> > >> Regards, > >> C.Palminha > >> > >>> > >>>> Signed-off-by: Carlos Palminha > >>>> --- > >>>> drivers/gpu/drm/i2c/adv7511.c | 8 ++++++++ > >>>> 1 file changed, 8 insertions(+) > >>>> > >>>> diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511.c > >>>> index 533d1e3..90082d2 100644 > >>>> --- a/drivers/gpu/drm/i2c/adv7511.c > >>>> +++ b/drivers/gpu/drm/i2c/adv7511.c > >>>> @@ -648,6 +648,13 @@ adv7511_encoder_detect(struct drm_encoder *encoder, > >>>> return status; > >>>> } > >>>> > >>>> +static bool adv7511_encoder_mode_fixup(struct drm_encoder *encoder, > >>>> + const struct drm_display_mode *mode, > >>>> + struct drm_display_mode *adjusted_mode) > >>>> +{ > >>>> + return true; > >>>> +} > >>>> + > >>>> static int adv7511_encoder_mode_valid(struct drm_encoder *encoder, > >>>> struct drm_display_mode *mode) > >>>> { > >>>> @@ -754,6 +761,7 @@ static void adv7511_encoder_mode_set(struct drm_encoder > >>>> *encoder, > >>>> > >>>> static const struct drm_encoder_slave_funcs adv7511_encoder_funcs = { > >>>> .dpms = adv7511_encoder_dpms, > >>>> + .mode_fixup = adv7511_encoder_mode_fixup, > >>>> .mode_valid = adv7511_encoder_mode_valid, > >>>> .mode_set = adv7511_encoder_mode_set, > >>>> .detect = adv7511_encoder_detect, > >>> > >> > >> > >> On 29-01-2016 17:48, Laurent Pinchart wrote: > >>> Hi Carlos, > >>> > >>> Thank you for the patch. > >>> > >>> On Friday 29 January 2016 10:33:47 Carlos Palminha wrote: > >>>> The mode_fixup is necessary when using it in a DRM FB driver pipeline. > >>> > >>> Instead of implementing stubs in encoder drivers, wouldn't it be better to > >>> make mode_fixup optional ? > >>> > >>>> Signed-off-by: Carlos Palminha > >>>> --- > >>>> drivers/gpu/drm/i2c/adv7511.c | 8 ++++++++ > >>>> 1 file changed, 8 insertions(+) > >>>> > >>>> diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511.c > >>>> index 533d1e3..90082d2 100644 > >>>> --- a/drivers/gpu/drm/i2c/adv7511.c > >>>> +++ b/drivers/gpu/drm/i2c/adv7511.c > >>>> @@ -648,6 +648,13 @@ adv7511_encoder_detect(struct drm_encoder *encoder, > >>>> return status; > >>>> } > >>>> > >>>> +static bool adv7511_encoder_mode_fixup(struct drm_encoder *encoder, > >>>> + const struct drm_display_mode *mode, > >>>> + struct drm_display_mode *adjusted_mode) > >>>> +{ > >>>> + return true; > >>>> +} > >>>> + > >>>> static int adv7511_encoder_mode_valid(struct drm_encoder *encoder, > >>>> struct drm_display_mode *mode) > >>>> { > >>>> @@ -754,6 +761,7 @@ static void adv7511_encoder_mode_set(struct drm_encoder > >>>> *encoder, > >>>> > >>>> static const struct drm_encoder_slave_funcs adv7511_encoder_funcs = { > >>>> .dpms = adv7511_encoder_dpms, > >>>> + .mode_fixup = adv7511_encoder_mode_fixup, > >>>> .mode_valid = adv7511_encoder_mode_valid, > >>>> .mode_set = adv7511_encoder_mode_set, > >>>> .detect = adv7511_encoder_detect, > >>> > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch