From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH v4 3/4] drm/mediatek: Add gamma correction. Date: Thu, 11 Aug 2016 10:51:05 +0200 Message-ID: <20160811085105.GI6232@phenom.ffwll.local> References: <1469672575-5847-1-git-send-email-bibby.hsieh@mediatek.com> <1469672575-5847-4-git-send-email-bibby.hsieh@mediatek.com> <1470900779.2493.20.camel@pengutronix.de> <20160811074410.GF4329@intel.com> <1470901876.2493.24.camel@pengutronix.de> <20160811080227.GH4329@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20160811080227.GH4329@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: Daniel Vetter , Cawa Cheng , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, Matthias Brugger , Yingjoe Chen , Mao Huang , Sascha Hauer , linux-arm-kernel@lists.infradead.org List-Id: linux-mediatek@lists.infradead.org T24gVGh1LCBBdWcgMTEsIDIwMTYgYXQgMTE6MDI6MjdBTSArMDMwMCwgVmlsbGUgU3lyasOkbMOk IHdyb3RlOgo+IE9uIFRodSwgQXVnIDExLCAyMDE2IGF0IDA5OjUxOjE2QU0gKzAyMDAsIFBoaWxp cHAgWmFiZWwgd3JvdGU6Cj4gPiBBbSBEb25uZXJzdGFnLCBkZW4gMTEuMDguMjAxNiwgMTA6NDQg KzAzMDAgc2NocmllYiBWaWxsZSBTeXJqw6Rsw6Q6Cj4gPiA+IE9uIFRodSwgQXVnIDExLCAyMDE2 IGF0IDA5OjMyOjU5QU0gKzAyMDAsIFBoaWxpcHAgWmFiZWwgd3JvdGU6Cj4gPiA+ID4gQW0gRG9u bmVyc3RhZywgZGVuIDI4LjA3LjIwMTYsIDEwOjIyICswODAwIHNjaHJpZWIgQmliYnkgSHNpZWg6 Cj4gPiA+ID4gPiBBZGQgZ2FtbWEgc2V0IGZ1bmN0aW9uIHRvIGNvcnJlY3QgYnJpZ2h0bmVzcyB2 YWx1ZXMuCj4gPiA+ID4gPiBJdCBhcHBsaWVzIGFyYml0cmFyeSBtYXBwaW5nIGN1cnZlIHRvIGNv bXBlbnNhdGUgdGhlCj4gPiA+ID4gPiBpbmNvcnJlY3QgdHJhbnNmZXIgZnVuY3Rpb24gb2YgdGhl IHBhbmVsLgo+ID4gPiA+ID4gCj4gPiA+ID4gPiBTaWduZWQtb2ZmLWJ5OiBCaWJieSBIc2llaCA8 YmliYnkuaHNpZWhAbWVkaWF0ZWsuY29tPgo+ID4gPiA+ID4gLS0tCj4gPiA+ID4gPiAgZHJpdmVy cy9ncHUvZHJtL21lZGlhdGVrL210a19kcm1fY3J0Yy5jICAgICB8ICAgIDggKysrKysrLQo+ID4g PiA+ID4gIGRyaXZlcnMvZ3B1L2RybS9tZWRpYXRlay9tdGtfZHJtX2NydGMuaCAgICAgfCAgICAx ICsKPiA+ID4gPiA+ICBkcml2ZXJzL2dwdS9kcm0vbWVkaWF0ZWsvbXRrX2RybV9kZHBfY29tcC5j IHwgICAzMSArKysrKysrKysrKysrKysrKysrKysrKysrKysKPiA+ID4gPiA+ICBkcml2ZXJzL2dw dS9kcm0vbWVkaWF0ZWsvbXRrX2RybV9kZHBfY29tcC5oIHwgICAxMCArKysrKysrKysKPiA+ID4g PiA+ICA0IGZpbGVzIGNoYW5nZWQsIDQ5IGluc2VydGlvbnMoKyksIDEgZGVsZXRpb24oLSkKPiA+ ID4gPiA+IAo+ID4gPiA+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9tZWRpYXRlay9t dGtfZHJtX2NydGMuYyBiL2RyaXZlcnMvZ3B1L2RybS9tZWRpYXRlay9tdGtfZHJtX2NydGMuYwo+ ID4gPiA+ID4gaW5kZXggMjRhYTNiYS4uY2JiNDYwYTUgMTAwNjQ0Cj4gPiA+ID4gPiAtLS0gYS9k cml2ZXJzL2dwdS9kcm0vbWVkaWF0ZWsvbXRrX2RybV9jcnRjLmMKPiA+ID4gPiA+ICsrKyBiL2Ry aXZlcnMvZ3B1L2RybS9tZWRpYXRlay9tdGtfZHJtX2NydGMuYwo+ID4gPiA+ID4gQEAgLTQwOSw2 ICs0MDksOSBAQCBzdGF0aWMgdm9pZCBtdGtfZHJtX2NydGNfYXRvbWljX2ZsdXNoKHN0cnVjdCBk cm1fY3J0YyAqY3J0YywKPiA+ID4gPiA+ICAJfQo+ID4gPiA+ID4gIAlpZiAocGVuZGluZ19wbGFu ZXMpCj4gPiA+ID4gPiAgCQltdGtfY3J0Yy0+cGVuZGluZ19wbGFuZXMgPSB0cnVlOwo+ID4gPiA+ ID4gKwlpZiAoY3J0Yy0+c3RhdGUtPmNvbG9yX21nbXRfY2hhbmdlZCkKPiA+ID4gPiA+ICsJCWZv ciAoaSA9IDA7IGkgPCBtdGtfY3J0Yy0+ZGRwX2NvbXBfbnI7IGkrKykKPiA+ID4gPiA+ICsJCQlt dGtfZGRwX2dhbW1hX3NldChtdGtfY3J0Yy0+ZGRwX2NvbXBbaV0sIGNydGMtPnN0YXRlKTsKPiA+ ID4gPiA+ICB9Cj4gPiA+ID4gPiAgCj4gPiA+ID4gPiAgc3RhdGljIGNvbnN0IHN0cnVjdCBkcm1f Y3J0Y19mdW5jcyBtdGtfY3J0Y19mdW5jcyA9IHsKPiA+ID4gPiA+IEBAIC00MTgsNiArNDIxLDcg QEAgc3RhdGljIGNvbnN0IHN0cnVjdCBkcm1fY3J0Y19mdW5jcyBtdGtfY3J0Y19mdW5jcyA9IHsK PiA+ID4gPiA+ICAJLnJlc2V0CQkJPSBtdGtfZHJtX2NydGNfcmVzZXQsCj4gPiA+ID4gPiAgCS5h dG9taWNfZHVwbGljYXRlX3N0YXRlCT0gbXRrX2RybV9jcnRjX2R1cGxpY2F0ZV9zdGF0ZSwKPiA+ ID4gPiA+ICAJLmF0b21pY19kZXN0cm95X3N0YXRlCT0gbXRrX2RybV9jcnRjX2Rlc3Ryb3lfc3Rh dGUsCj4gPiA+ID4gPiArCS5nYW1tYV9zZXQJCT0gZHJtX2F0b21pY19oZWxwZXJfbGVnYWN5X2dh bW1hX3NldCwKPiA+ID4gPiA+ICB9Owo+ID4gPiA+ID4gIAo+ID4gPiA+ID4gIHN0YXRpYyBjb25z dCBzdHJ1Y3QgZHJtX2NydGNfaGVscGVyX2Z1bmNzIG10a19jcnRjX2hlbHBlcl9mdW5jcyA9IHsK PiA+ID4gPiA+IEBAIC01NjgsNyArNTcyLDkgQEAgaW50IG10a19kcm1fY3J0Y19jcmVhdGUoc3Ry dWN0IGRybV9kZXZpY2UgKmRybV9kZXYsCj4gPiA+ID4gPiAgCQkJCSZtdGtfY3J0Yy0+cGxhbmVz WzFdLmJhc2UsIHBpcGUpOwo+ID4gPiA+ID4gIAlpZiAocmV0IDwgMCkKPiA+ID4gPiA+ICAJCWdv dG8gdW5wcmVwYXJlOwo+ID4gPiA+ID4gLQo+ID4gPiA+ID4gKwlkcm1fbW9kZV9jcnRjX3NldF9n YW1tYV9zaXplKCZtdGtfY3J0Yy0+YmFzZSwgTVRLX0xVVF9TSVpFKTsKPiA+ID4gPiA+ICsJZHJt X2hlbHBlcl9jcnRjX2VuYWJsZV9jb2xvcl9tZ210KCZtdGtfY3J0Yy0+YmFzZSwgTVRLX0xVVF9T SVpFLAo+ID4gPiA+ID4gKwkJCQkJICBNVEtfTFVUX1NJWkUpOwo+ID4gPiA+IAo+ID4gPiA+IEkg aGF2ZSBhcHBsaWVkIGFsbCBmb3VyIHBhdGNoZXMgYW5kIHJlYmFzZWQgb250byB2NC44LXJjMSwg cmVwbGFjaW5nCj4gPiA+ID4gZHJtX2hlbHBlcl9jcnRjX2VuYWJsZV9jb2xvcl9tZ210IHdpdGg6 Cj4gPiA+ID4gCj4gPiA+ID4gCWRybV9jcnRjX2VuYWJsZV9jb2xvcl9tZ210KCZtdGtfY3J0Yy0+ YmFzZSwgTVRLX0xVVF9TSVpFLAo+ID4gPiA+IAkJCQkgICB0cnVlLCBNVEtfTFVUX1NJWkUpOwo+ ID4gPiAKPiA+ID4gQlRXIHRoYXQgbG9va3Mgd3JvbmcgKGFscmVhZHkgaW4gdGhlIG9yaWdpbmFs KS4gQUZBSUNTIHRoZSBwYXRjaCBqdXN0Cj4gPiA+IGhhbmRsZWQgdGhlIGdhbW1hX2x1dCwgbm90 IHRoZSBkZWdhbW1hX2x1dCwgc28gdGVsbGluZyB5b3UgaGF2ZSBib3RoCj4gPiA+IGlzIG5vdCBy aWdodC4KPiA+IAo+ID4gVGhhbmtzLCBzbyBzaG91bGQgdGhhdCBiZQo+ID4gICAgICAgIGRybV9j cnRjX2VuYWJsZV9jb2xvcl9tZ210KCZtdGtfY3J0Yy0+YmFzZSwgMCwgZmFsc2UsCj4gPiAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgTVRLX0xVVF9TSVpFKTsKPiA+IGluc3RlYWQs IHNpbmNlIHdlIG9ubHkgaGFuZGxlIGdhbW1hPwo+IAo+IEhtbS4gWWVhaCwgdGhhdCBsb29rcyBj b3JyZWN0IHNpbmNlIHlvdSBkaWRuJ3Qgc2VlbSB0byBoYXZlICJjdG0iIGVpdGhlci4KCll1cCwg dGhhdCdzIGhvdyB0aGlzIGlzIG1lYW50IHRvIGJlIHVzZWQuCi1EYW5pZWwKLS0gCkRhbmllbCBW ZXR0ZXIKU29mdHdhcmUgRW5naW5lZXIsIEludGVsIENvcnBvcmF0aW9uCmh0dHA6Ly9ibG9nLmZm d2xsLmNoCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRy aS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRw czovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: daniel@ffwll.ch (Daniel Vetter) Date: Thu, 11 Aug 2016 10:51:05 +0200 Subject: [PATCH v4 3/4] drm/mediatek: Add gamma correction. In-Reply-To: <20160811080227.GH4329@intel.com> References: <1469672575-5847-1-git-send-email-bibby.hsieh@mediatek.com> <1469672575-5847-4-git-send-email-bibby.hsieh@mediatek.com> <1470900779.2493.20.camel@pengutronix.de> <20160811074410.GF4329@intel.com> <1470901876.2493.24.camel@pengutronix.de> <20160811080227.GH4329@intel.com> Message-ID: <20160811085105.GI6232@phenom.ffwll.local> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, Aug 11, 2016 at 11:02:27AM +0300, Ville Syrj?l? wrote: > On Thu, Aug 11, 2016 at 09:51:16AM +0200, Philipp Zabel wrote: > > Am Donnerstag, den 11.08.2016, 10:44 +0300 schrieb Ville Syrj?l?: > > > On Thu, Aug 11, 2016 at 09:32:59AM +0200, Philipp Zabel wrote: > > > > Am Donnerstag, den 28.07.2016, 10:22 +0800 schrieb Bibby Hsieh: > > > > > Add gamma set function to correct brightness values. > > > > > It applies arbitrary mapping curve to compensate the > > > > > incorrect transfer function of the panel. > > > > > > > > > > Signed-off-by: Bibby Hsieh > > > > > --- > > > > > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 8 ++++++- > > > > > drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 1 + > > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 31 +++++++++++++++++++++++++++ > > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 10 +++++++++ > > > > > 4 files changed, 49 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > > > > index 24aa3ba..cbb460a5 100644 > > > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > > > > @@ -409,6 +409,9 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc, > > > > > } > > > > > if (pending_planes) > > > > > mtk_crtc->pending_planes = true; > > > > > + if (crtc->state->color_mgmt_changed) > > > > > + for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) > > > > > + mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state); > > > > > } > > > > > > > > > > static const struct drm_crtc_funcs mtk_crtc_funcs = { > > > > > @@ -418,6 +421,7 @@ static const struct drm_crtc_funcs mtk_crtc_funcs = { > > > > > .reset = mtk_drm_crtc_reset, > > > > > .atomic_duplicate_state = mtk_drm_crtc_duplicate_state, > > > > > .atomic_destroy_state = mtk_drm_crtc_destroy_state, > > > > > + .gamma_set = drm_atomic_helper_legacy_gamma_set, > > > > > }; > > > > > > > > > > static const struct drm_crtc_helper_funcs mtk_crtc_helper_funcs = { > > > > > @@ -568,7 +572,9 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev, > > > > > &mtk_crtc->planes[1].base, pipe); > > > > > if (ret < 0) > > > > > goto unprepare; > > > > > - > > > > > + drm_mode_crtc_set_gamma_size(&mtk_crtc->base, MTK_LUT_SIZE); > > > > > + drm_helper_crtc_enable_color_mgmt(&mtk_crtc->base, MTK_LUT_SIZE, > > > > > + MTK_LUT_SIZE); > > > > > > > > I have applied all four patches and rebased onto v4.8-rc1, replacing > > > > drm_helper_crtc_enable_color_mgmt with: > > > > > > > > drm_crtc_enable_color_mgmt(&mtk_crtc->base, MTK_LUT_SIZE, > > > > true, MTK_LUT_SIZE); > > > > > > BTW that looks wrong (already in the original). AFAICS the patch just > > > handled the gamma_lut, not the degamma_lut, so telling you have both > > > is not right. > > > > Thanks, so should that be > > drm_crtc_enable_color_mgmt(&mtk_crtc->base, 0, false, > > MTK_LUT_SIZE); > > instead, since we only handle gamma? > > Hmm. Yeah, that looks correct since you didn't seem to have "ctm" either. Yup, that's how this is meant to be used. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932885AbcHKIwL (ORCPT ); Thu, 11 Aug 2016 04:52:11 -0400 Received: from mail-wm0-f67.google.com ([74.125.82.67]:33615 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932491AbcHKIvK (ORCPT ); Thu, 11 Aug 2016 04:51:10 -0400 Date: Thu, 11 Aug 2016 10:51:05 +0200 From: Daniel Vetter To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: Philipp Zabel , Bibby Hsieh , linux-kernel@vger.kernel.org, Daniel Vetter , Cawa Cheng , dri-devel@lists.freedesktop.org, Mao Huang , linux-mediatek@lists.infradead.org, Sascha Hauer , Matthias Brugger , Yingjoe Chen , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v4 3/4] drm/mediatek: Add gamma correction. Message-ID: <20160811085105.GI6232@phenom.ffwll.local> Mail-Followup-To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= , Philipp Zabel , Bibby Hsieh , linux-kernel@vger.kernel.org, Cawa Cheng , dri-devel@lists.freedesktop.org, Mao Huang , linux-mediatek@lists.infradead.org, Sascha Hauer , Matthias Brugger , Yingjoe Chen , linux-arm-kernel@lists.infradead.org References: <1469672575-5847-1-git-send-email-bibby.hsieh@mediatek.com> <1469672575-5847-4-git-send-email-bibby.hsieh@mediatek.com> <1470900779.2493.20.camel@pengutronix.de> <20160811074410.GF4329@intel.com> <1470901876.2493.24.camel@pengutronix.de> <20160811080227.GH4329@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20160811080227.GH4329@intel.com> X-Operating-System: Linux phenom 4.6.0-1-amd64 User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Aug 11, 2016 at 11:02:27AM +0300, Ville Syrjälä wrote: > On Thu, Aug 11, 2016 at 09:51:16AM +0200, Philipp Zabel wrote: > > Am Donnerstag, den 11.08.2016, 10:44 +0300 schrieb Ville Syrjälä: > > > On Thu, Aug 11, 2016 at 09:32:59AM +0200, Philipp Zabel wrote: > > > > Am Donnerstag, den 28.07.2016, 10:22 +0800 schrieb Bibby Hsieh: > > > > > Add gamma set function to correct brightness values. > > > > > It applies arbitrary mapping curve to compensate the > > > > > incorrect transfer function of the panel. > > > > > > > > > > Signed-off-by: Bibby Hsieh > > > > > --- > > > > > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 8 ++++++- > > > > > drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 1 + > > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 31 +++++++++++++++++++++++++++ > > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 10 +++++++++ > > > > > 4 files changed, 49 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > > > > index 24aa3ba..cbb460a5 100644 > > > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > > > > @@ -409,6 +409,9 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc, > > > > > } > > > > > if (pending_planes) > > > > > mtk_crtc->pending_planes = true; > > > > > + if (crtc->state->color_mgmt_changed) > > > > > + for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) > > > > > + mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state); > > > > > } > > > > > > > > > > static const struct drm_crtc_funcs mtk_crtc_funcs = { > > > > > @@ -418,6 +421,7 @@ static const struct drm_crtc_funcs mtk_crtc_funcs = { > > > > > .reset = mtk_drm_crtc_reset, > > > > > .atomic_duplicate_state = mtk_drm_crtc_duplicate_state, > > > > > .atomic_destroy_state = mtk_drm_crtc_destroy_state, > > > > > + .gamma_set = drm_atomic_helper_legacy_gamma_set, > > > > > }; > > > > > > > > > > static const struct drm_crtc_helper_funcs mtk_crtc_helper_funcs = { > > > > > @@ -568,7 +572,9 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev, > > > > > &mtk_crtc->planes[1].base, pipe); > > > > > if (ret < 0) > > > > > goto unprepare; > > > > > - > > > > > + drm_mode_crtc_set_gamma_size(&mtk_crtc->base, MTK_LUT_SIZE); > > > > > + drm_helper_crtc_enable_color_mgmt(&mtk_crtc->base, MTK_LUT_SIZE, > > > > > + MTK_LUT_SIZE); > > > > > > > > I have applied all four patches and rebased onto v4.8-rc1, replacing > > > > drm_helper_crtc_enable_color_mgmt with: > > > > > > > > drm_crtc_enable_color_mgmt(&mtk_crtc->base, MTK_LUT_SIZE, > > > > true, MTK_LUT_SIZE); > > > > > > BTW that looks wrong (already in the original). AFAICS the patch just > > > handled the gamma_lut, not the degamma_lut, so telling you have both > > > is not right. > > > > Thanks, so should that be > > drm_crtc_enable_color_mgmt(&mtk_crtc->base, 0, false, > > MTK_LUT_SIZE); > > instead, since we only handle gamma? > > Hmm. Yeah, that looks correct since you didn't seem to have "ctm" either. Yup, that's how this is meant to be used. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch