On Mon, 08 Dec 2014, "Singh, Gaurav K" <gaurav.k.singh@xxxxxxxxx> wrote:
> On 12/8/2014 5:07 PM, Jani Nikula wrote:
>> On Sun, 07 Dec 2014, Gaurav K Singh <gaurav.k.singh@xxxxxxxxx> 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@xxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxxxxxxxxxxxxxx
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
