Re: [PATCH 0/1] drm/i915: connector specific backlight hooks for eDP/DSI

From: Jani Nikula <jani.nikula@intel.com>
To: m.deepak@intel.com, yetundex.adebisi@intel.com,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 0/1] drm/i915: connector specific backlight hooks for eDP/DSI
Date: Mon, 14 Sep 2015 15:29:13 +0300	[thread overview]
Message-ID: <87mvwpjiau.fsf@intel.com> (raw)
In-Reply-To: <cover.1442227790.git.jani.nikula@intel.com>

On Mon, 14 Sep 2015, Jani Nikula <jani.nikula@intel.com> wrote:
> Hi all -
>
> This patch moves the backlight hooks from dev_priv->display to
> intel_panel. This should enable connector specific backlight control
> mechanisms, such as DPCD for eDP and DCS commands for DSI, to be neatly
> connected to the existing backlight infrastructure.
>
> Basically it should be sufficient to just point the hooks at the
> specific backlight control mechanisms, and let the generic code be. You
> can still add those mechanisms to separate files. There should be very
> little to no special casing anywhere, apart from the hooks themselves.
>
> This is in response to the two new backlight control mechanisms in
> flight, for eDP [1] and DSI [2]. We should not quickly bolt on these
> mechanisms when there's a better way with not much extra effort.

To be more specific, you need to define a set of functions for eDP:

int intel_dp_aux_setup_backlight(struct intel_connector *connector,
	enum pipe pipe);
uint32_t intel_dp_aux_get_backlight(struct intel_connector *connector);
void intel_dp_aux_set_backlight(struct intel_connector *connector,
	uint32_t level);
void intel_dp_aux_disable_backlight(struct intel_connector *connector);
void intel_dp_aux_enable_backlight(struct intel_connector *connector);

and another set of functions for DSI:

int intel_dsi_dcs_setup_backlight(struct intel_connector *connector,
	enum pipe pipe);
uint32_t intel_dsi_dcs_get_backlight(struct intel_connector *connector);
void intel_dsi_dcs_set_backlight(struct intel_connector *connector,
	uint32_t level);
void intel_dsi_dcs_disable_backlight(struct intel_connector *connector);
void intel_dsi_dcs_enable_backlight(struct intel_connector *connector);

and set these up in intel_panel_init_backlight_funcs, depending on the
connector type and platform and VBT etc. Otherwise, the changes to
intel_dp.c or intel_dsi.c or intel_panel.c should be non-existent or
very minimal.

If (and only if) there's a need to have more than one connector with
backlight control active at the same time, we need to tweak
intel_panel.c a bit. But that should be a separate patch. And even so,
IMO the naming could be just intel_backlightN where N is a number.

BR,
Jani.

>
> This patch is untested, but there's no rocket science here. I just
> wanted this quickly out.
>
> BR,
> Jani.
>
>
> [1] http://mid.gmane.org/1441894302-28475-1-git-send-email-yetundex.adebisi@intel.com
> [2] http://mid.gmane.org/1441946868-4985-1-git-send-email-m.deepak@intel.com
>
>
> Jani Nikula (1):
>   drm/i915: make backlight hooks connector specific
>
>  drivers/gpu/drm/i915/i915_drv.h      |   9 ---
>  drivers/gpu/drm/i915/intel_display.c |   2 -
>  drivers/gpu/drm/i915/intel_dp.c      |   2 +-
>  drivers/gpu/drm/i915/intel_drv.h     |  13 +++-
>  drivers/gpu/drm/i915/intel_panel.c   | 116 +++++++++++++++++++----------------
>  5 files changed, 74 insertions(+), 68 deletions(-)
>
> -- 
> 2.1.4
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx