[Intel-gfx] [PATCH 4/4] drm/i915: Get HW state changes required for DSI port C

[Jani Nikula]

On Mon, 08 Dec 2014, "Singh, Gaurav K" <gaurav.k.singh at intel.com> wrote:
> On 12/8/2014 5:07 PM, Jani Nikula wrote:
>> On Sun, 07 Dec 2014, Gaurav K Singh <gaurav.k.singh at intel.com> wrote:
>>> Due to some hardware limitations, MIPI Port C DPI Enable bit
>>> does not get set. To check whether DSI Port C was enabled in BIOS,
>>> check the Pipe B enable bit for DSI Port C. In hardware, DSI Port C
>>> is linked with Pipe B.
>> "due to some hardware limitations" is awfully vague. The same code will
>> be used on many platforms; I doubt this applies to all of them. Is there
>> a workaround name for this?
>>
>> BR,
>> Jani.
>
> Hi Jani,
>
> This is currently only for BYT, I missed to add in this patch commit header.
>
> Software workaround for getting the HW status of DSI Port C on BYT, will 
> it be fine if I include this in the commit header.

No, it's not enough to amend the commit message. I prefer the
implementation to be generic for all platforms, and quirks like this
need to be special cases isolated as much as possible.

Please do something along the lines of:

	port_ctl = I915_READ(MIPI_PORT_CTRL(port));
	func = I915_READ(MIPI_DSI_FUNC_PRG(port));

	dpi_enabled = port_ctl & DPI_ENABLE;

        /* Due to some BYT hardware limitation ... etc... */
        if (IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev) && port == PORT_C)
		dpi_enabled = I915_READ(PIPECONF(PIPE_B)) & PIPECONF_ENABLE;

	if (dpi_enabled || (func & CMD_MODE_DATA_WIDTH_MASK))

	/* etc */

As you see, the exception clearly stands out, it's specific to BYT, and
it's easy to amend the condition or rip it out altogether as needed.

BR,
Jani.

>
> With regards,
> Gaurav
>>
>>> Signed-off-by: Gaurav K Singh <gaurav.k.singh at intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_dsi.c |   21 ++++++++++++++++-----
>>>   1 file changed, 16 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>>> index 215d004..0334c4d 100644
>>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>>> @@ -398,8 +398,9 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>>>   				   enum pipe *pipe)
>>>   {
>>>   	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>>> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>>>   	enum intel_display_power_domain power_domain;
>>> -	u32 port_ctl, func;
>>> +	u32 dsi_status, func;
>>>   	enum port port;
>>>   
>>>   	DRM_DEBUG_KMS("\n");
>>> @@ -409,13 +410,23 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>>>   		return false;
>>>   
>>>   	/* XXX: this only works for one DSI output */
>>> -	for_each_dsi_port(port, (1 << PORT_A) | (1 << PORT_C)) {
>>> -		port_ctl = I915_READ(MIPI_PORT_CTRL(port));
>>> +	for_each_dsi_port(port, intel_dsi->ports) {
>>>   		func = I915_READ(MIPI_DSI_FUNC_PRG(port));
>>>   
>>> -		if ((port_ctl & DPI_ENABLE) || (func & CMD_MODE_DATA_WIDTH_MASK)) {
>>> +		/* Due to some hardware limitations, MIPI Port C DPI Enable
>>> +		 * bit does not get set. To check whether DSI Port C was
>>> +		 * enabled in BIOS, check the Pipe B enable bit
>>> +		 */
>>> +		if (port == PORT_C)
>>> +			dsi_status = I915_READ(PIPECONF(PIPE_B)) &
>>> +							PIPECONF_ENABLE;
>>> +		else
>>> +			dsi_status = I915_READ(MIPI_PORT_CTRL(port)) &
>>> +							DPI_ENABLE;
>>> +
>>> +		if (dsi_status || (func & CMD_MODE_DATA_WIDTH_MASK)) {
>>>   			if (I915_READ(MIPI_DEVICE_READY(port)) & DEVICE_READY) {
>>> -				*pipe = port == PORT_A ? PIPE_A : PIPE_C;
>>> +				*pipe = port == PORT_A ? PIPE_A : PIPE_B;
>>>   				return true;
>>>   			}
>>>   		}
>>> -- 
>>> 1.7.9.5
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

-- 
Jani Nikula, Intel Open Source Technology Center