All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mihail Atanassov <Mihail.Atanassov@arm.com>
To: "james qian wang (Arm Technology China)" <james.qian.wang@arm.com>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>,
	"airlied@linux.ie" <airlied@linux.ie>,
	Brian Starkey <Brian.Starkey@arm.com>,
	"maarten.lankhorst@linux.intel.com" 
	<maarten.lankhorst@linux.intel.com>,
	"sean@poorly.run" <sean@poorly.run>,
	"imirkin@alum.mit.edu" <imirkin@alum.mit.edu>,
	"Jonathan Chai (Arm Technology China)" <Jonathan.Chai@arm.com>,
	"Julien Yin (Arm Technology China)" <Julien.Yin@arm.com>,
	"Thomas Sun (Arm Technology China)" <thomas.Sun@arm.com>,
	"Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>,
	Ayan Halder <Ayan.Halder@arm.com>,
	"Tiannan Zhu (Arm Technology China)" <Tiannan.Zhu@arm.com>,
	"Yiqi Kang (Arm Technology China)" <Yiqi.Kang@arm.com>,
	nd <nd@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>, Ben Davis <Ben.Davis@arm.com>,
	"Oscar Zhang (Arm Technology China)" <Oscar.Zhang@arm.com>,
	"Channing Chen (Arm Technology China)" <Channing.Chen@arm.com>
Subject: Re: [PATCH v2 4/4] drm/komeda: Adds gamma and color-transform support for DOU-IPS
Date: Fri, 11 Oct 2019 08:54:03 +0000	[thread overview]
Message-ID: <8416585.jh9292Gg6g@e123338-lin> (raw)
In-Reply-To: <20191011054459.17984-5-james.qian.wang@arm.com>

Hi James, Lowry,

On Friday, 11 October 2019 06:45:50 BST james qian wang (Arm Technology China) wrote:
> From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> 
> Adds gamma and color-transform support for DOU-IPS.
> Adds two caps members fgamma_coeffs and ctm_coeffs to komeda_improc_state.
> If color management changed, set gamma and color-transform accordingly.
> 
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> ---
>  .../arm/display/komeda/d71/d71_component.c    | 24 +++++++++++++++++++
>  .../gpu/drm/arm/display/komeda/komeda_crtc.c  |  2 ++
>  .../drm/arm/display/komeda/komeda_pipeline.h  |  3 +++
>  .../display/komeda/komeda_pipeline_state.c    |  6 +++++
>  4 files changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> index c3d29c0b051b..e7e5a8e4430e 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> @@ -942,15 +942,39 @@ static int d71_merger_init(struct d71_dev *d71,
>  static void d71_improc_update(struct komeda_component *c,
>  			      struct komeda_component_state *state)
>  {
> +	struct drm_crtc_state *crtc_st = state->crtc->state;
I'm not sure it's a good idea to introduce a dependency on drm state
so far down in the HW funcs, is there a good reason for the direct prod?
>  	struct komeda_improc_state *st = to_improc_st(state);
> +	struct d71_pipeline *pipe = to_d71_pipeline(c->pipeline);
>  	u32 __iomem *reg = c->reg;
>  	u32 index;
> +	u32 mask = 0, ctrl = 0;
>  
>  	for_each_changed_input(state, index)
>  		malidp_write32(reg, BLK_INPUT_ID0 + index * 4,
>  			       to_d71_input_id(state, index));
>  
>  	malidp_write32(reg, BLK_SIZE, HV_SIZE(st->hsize, st->vsize));
> +
> +	if (crtc_st->color_mgmt_changed) {
> +		mask |= IPS_CTRL_FT | IPS_CTRL_RGB;
> +
> +		if (crtc_st->gamma_lut) {
> +			malidp_write_group(pipe->dou_ft_coeff_addr, FT_COEFF0,
> +					   KOMEDA_N_GAMMA_COEFFS,
> +					   st->fgamma_coeffs);
> +			ctrl |= IPS_CTRL_FT; /* enable gamma */
> +		}
> +
> +		if (crtc_st->ctm) {
> +			malidp_write_group(reg, IPS_RGB_RGB_COEFF0,
> +					   KOMEDA_N_CTM_COEFFS,
> +					   st->ctm_coeffs);
> +			ctrl |= IPS_CTRL_RGB; /* enable gamut */
> +		}
> +	}
> +
> +	if (mask)
> +		malidp_write32_mask(reg, BLK_CONTROL, mask, ctrl);
>  }
>  
>  static void d71_improc_dump(struct komeda_component *c, struct seq_file *sf)
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index 9beeda04818b..406b9d0ca058 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -590,6 +590,8 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms,
>  
>  	crtc->port = kcrtc->master->of_output_port;
>  
> +	drm_crtc_enable_color_mgmt(crtc, 0, true, KOMEDA_COLOR_LUT_SIZE);
> +
>  	return err;
>  }
>  
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> index b322f52ba8f2..c5ab8096c85d 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> @@ -11,6 +11,7 @@
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
>  #include "malidp_utils.h"
> +#include "komeda_color_mgmt.h"
>  
>  #define KOMEDA_MAX_PIPELINES		2
>  #define KOMEDA_PIPELINE_MAX_LAYERS	4
> @@ -324,6 +325,8 @@ struct komeda_improc {
>  struct komeda_improc_state {
>  	struct komeda_component_state base;
>  	u16 hsize, vsize;
> +	u32 fgamma_coeffs[KOMEDA_N_GAMMA_COEFFS];
> +	u32 ctm_coeffs[KOMEDA_N_CTM_COEFFS];
>  };
>  
>  /* display timing controller */
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> index 0ba9c6aa3708..4a40b37eb1a6 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> @@ -756,6 +756,12 @@ komeda_improc_validate(struct komeda_improc *improc,
>  	st->hsize = dflow->in_w;
>  	st->vsize = dflow->in_h;
>  
> +	if (kcrtc_st->base.color_mgmt_changed) {
> +		drm_lut_to_fgamma_coeffs(kcrtc_st->base.gamma_lut,
> +					 st->fgamma_coeffs);
> +		drm_ctm_to_coeffs(kcrtc_st->base.ctm, st->ctm_coeffs);
> +	}
> +
>  	komeda_component_add_input(&st->base, &dflow->input, 0);
>  	komeda_component_set_output(&dflow->input, &improc->base, 0);
>  
> 


-- 
Mihail




WARNING: multiple messages have this Message-ID (diff)
From: Mihail Atanassov <Mihail.Atanassov@arm.com>
To: "james qian wang (Arm Technology China)" <james.qian.wang@arm.com>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>,
	"airlied@linux.ie" <airlied@linux.ie>,
	Brian Starkey <Brian.Starkey@arm.com>,
	"maarten.lankhorst@linux.intel.com"
	<maarten.lankhorst@linux.intel.com>,
	"sean@poorly.run" <sean@poorly.run>,
	"imirkin@alum.mit.edu" <imirkin@alum.mit.edu>,
	"Jonathan Chai (Arm Technology China)" <Jonathan.Chai@arm.com>,
	"Julien Yin (Arm Technology China)" <Julien.Yin@arm.com>,
	"Thomas Sun (Arm Technology China)" <thomas.Sun@arm.com>,
	"Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>,
	Ayan Halder <Ayan.Halder@arm.com>,
	"Tiannan Zhu (Arm Technology China)" <Tiannan.Zhu@arm.com>,
	"Yiqi Kang (Arm Technology China)" <Yiqi.Kang@arm.com>,
	nd <nd@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2 4/4] drm/komeda: Adds gamma and color-transform support for DOU-IPS
Date: Fri, 11 Oct 2019 08:54:03 +0000	[thread overview]
Message-ID: <8416585.jh9292Gg6g@e123338-lin> (raw)
In-Reply-To: <20191011054459.17984-5-james.qian.wang@arm.com>

Hi James, Lowry,

On Friday, 11 October 2019 06:45:50 BST james qian wang (Arm Technology China) wrote:
> From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> 
> Adds gamma and color-transform support for DOU-IPS.
> Adds two caps members fgamma_coeffs and ctm_coeffs to komeda_improc_state.
> If color management changed, set gamma and color-transform accordingly.
> 
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> ---
>  .../arm/display/komeda/d71/d71_component.c    | 24 +++++++++++++++++++
>  .../gpu/drm/arm/display/komeda/komeda_crtc.c  |  2 ++
>  .../drm/arm/display/komeda/komeda_pipeline.h  |  3 +++
>  .../display/komeda/komeda_pipeline_state.c    |  6 +++++
>  4 files changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> index c3d29c0b051b..e7e5a8e4430e 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> @@ -942,15 +942,39 @@ static int d71_merger_init(struct d71_dev *d71,
>  static void d71_improc_update(struct komeda_component *c,
>  			      struct komeda_component_state *state)
>  {
> +	struct drm_crtc_state *crtc_st = state->crtc->state;
I'm not sure it's a good idea to introduce a dependency on drm state
so far down in the HW funcs, is there a good reason for the direct prod?
>  	struct komeda_improc_state *st = to_improc_st(state);
> +	struct d71_pipeline *pipe = to_d71_pipeline(c->pipeline);
>  	u32 __iomem *reg = c->reg;
>  	u32 index;
> +	u32 mask = 0, ctrl = 0;
>  
>  	for_each_changed_input(state, index)
>  		malidp_write32(reg, BLK_INPUT_ID0 + index * 4,
>  			       to_d71_input_id(state, index));
>  
>  	malidp_write32(reg, BLK_SIZE, HV_SIZE(st->hsize, st->vsize));
> +
> +	if (crtc_st->color_mgmt_changed) {
> +		mask |= IPS_CTRL_FT | IPS_CTRL_RGB;
> +
> +		if (crtc_st->gamma_lut) {
> +			malidp_write_group(pipe->dou_ft_coeff_addr, FT_COEFF0,
> +					   KOMEDA_N_GAMMA_COEFFS,
> +					   st->fgamma_coeffs);
> +			ctrl |= IPS_CTRL_FT; /* enable gamma */
> +		}
> +
> +		if (crtc_st->ctm) {
> +			malidp_write_group(reg, IPS_RGB_RGB_COEFF0,
> +					   KOMEDA_N_CTM_COEFFS,
> +					   st->ctm_coeffs);
> +			ctrl |= IPS_CTRL_RGB; /* enable gamut */
> +		}
> +	}
> +
> +	if (mask)
> +		malidp_write32_mask(reg, BLK_CONTROL, mask, ctrl);
>  }
>  
>  static void d71_improc_dump(struct komeda_component *c, struct seq_file *sf)
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index 9beeda04818b..406b9d0ca058 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -590,6 +590,8 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms,
>  
>  	crtc->port = kcrtc->master->of_output_port;
>  
> +	drm_crtc_enable_color_mgmt(crtc, 0, true, KOMEDA_COLOR_LUT_SIZE);
> +
>  	return err;
>  }
>  
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> index b322f52ba8f2..c5ab8096c85d 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> @@ -11,6 +11,7 @@
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
>  #include "malidp_utils.h"
> +#include "komeda_color_mgmt.h"
>  
>  #define KOMEDA_MAX_PIPELINES		2
>  #define KOMEDA_PIPELINE_MAX_LAYERS	4
> @@ -324,6 +325,8 @@ struct komeda_improc {
>  struct komeda_improc_state {
>  	struct komeda_component_state base;
>  	u16 hsize, vsize;
> +	u32 fgamma_coeffs[KOMEDA_N_GAMMA_COEFFS];
> +	u32 ctm_coeffs[KOMEDA_N_CTM_COEFFS];
>  };
>  
>  /* display timing controller */
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> index 0ba9c6aa3708..4a40b37eb1a6 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> @@ -756,6 +756,12 @@ komeda_improc_validate(struct komeda_improc *improc,
>  	st->hsize = dflow->in_w;
>  	st->vsize = dflow->in_h;
>  
> +	if (kcrtc_st->base.color_mgmt_changed) {
> +		drm_lut_to_fgamma_coeffs(kcrtc_st->base.gamma_lut,
> +					 st->fgamma_coeffs);
> +		drm_ctm_to_coeffs(kcrtc_st->base.ctm, st->ctm_coeffs);
> +	}
> +
>  	komeda_component_add_input(&st->base, &dflow->input, 0);
>  	komeda_component_set_output(&dflow->input, &improc->base, 0);
>  
> 


-- 
Mihail

  reply	other threads:[~2019-10-11  8:54 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-11  5:45 [PATCH v2 0/4] drm/komeda: Enable CRTC color-mgmt james qian wang (Arm Technology China)
2019-10-11  5:45 ` james qian wang (Arm Technology China)
2019-10-11  5:45 ` [PATCH v2 1/4] drm: Add a new helper drm_color_ctm_s31_32_to_qm_n() james qian wang (Arm Technology China)
2019-10-11  5:45   ` james qian wang (Arm Technology China)
2019-10-11  8:26   ` Mihail Atanassov
2019-10-11  8:26     ` Mihail Atanassov
2019-10-14  9:43     ` james qian wang (Arm Technology China)
2019-10-14  9:43       ` james qian wang (Arm Technology China)
2019-10-14 10:39       ` Mihail Atanassov
2019-10-14 10:39         ` Mihail Atanassov
2019-10-11  5:45 ` [PATCH v2 2/4] drm/komeda: Add drm_lut_to_fgamma_coeffs() james qian wang (Arm Technology China)
2019-10-11  5:45   ` james qian wang (Arm Technology China)
2019-10-11  8:30   ` Mihail Atanassov
2019-10-11  8:30     ` Mihail Atanassov
2019-10-11  5:45 ` [PATCH v2 3/4] drm/komeda: Add drm_ctm_to_coeffs() james qian wang (Arm Technology China)
2019-10-11  5:45   ` james qian wang (Arm Technology China)
2019-10-11  8:37   ` Mihail Atanassov
2019-10-11  8:37     ` Mihail Atanassov
2019-10-11  5:45 ` [PATCH v2 4/4] drm/komeda: Adds gamma and color-transform support for DOU-IPS james qian wang (Arm Technology China)
2019-10-11  5:45   ` james qian wang (Arm Technology China)
2019-10-11  8:54   ` Mihail Atanassov [this message]
2019-10-11  8:54     ` Mihail Atanassov
2019-10-11 10:12     ` Lowry Li (Arm Technology China)
2019-10-11 10:12       ` Lowry Li (Arm Technology China)
2019-10-11 10:27       ` Mihail Atanassov
2019-10-11 10:27         ` Mihail Atanassov
  -- strict thread matches above, loose matches on Subject: below --
2019-10-11  5:43 [PATCH v2 0/4] drm/komeda: Enable CRTC color-mgmt james qian wang (Arm Technology China)
2019-10-11  5:43 ` [PATCH v2 4/4] drm/komeda: Adds gamma and color-transform support for DOU-IPS james qian wang (Arm Technology China)
2019-10-11  5:43   ` james qian wang (Arm Technology China)
2019-10-11  6:21   ` sandy.huang
2019-10-11  6:21     ` sandy.huang
2019-10-11  7:12     ` james qian wang (Arm Technology China)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8416585.jh9292Gg6g@e123338-lin \
    --to=mihail.atanassov@arm.com \
    --cc=Ayan.Halder@arm.com \
    --cc=Ben.Davis@arm.com \
    --cc=Brian.Starkey@arm.com \
    --cc=Channing.Chen@arm.com \
    --cc=Jonathan.Chai@arm.com \
    --cc=Julien.Yin@arm.com \
    --cc=Liviu.Dudau@arm.com \
    --cc=Lowry.Li@arm.com \
    --cc=Oscar.Zhang@arm.com \
    --cc=Tiannan.Zhu@arm.com \
    --cc=Yiqi.Kang@arm.com \
    --cc=airlied@linux.ie \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=imirkin@alum.mit.edu \
    --cc=james.qian.wang@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=nd@arm.com \
    --cc=sean@poorly.run \
    --cc=thomas.Sun@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.