Hi Dave, Thanks for your feedback. No issue found actually if I remove "volatile" on my platform. In my experience, if the value is volatile, adding "volatile" will be safer and no harm, that is why I add it by default. If you think it is not necessary, it's ok for me to remove it.
Regards, Y.C. Chen Dave Airlie <airl...@gmail.com> 於 2018年11月22日 週四 上午9:15寫道: > On Wed, 31 Oct 2018 at 18:12, Y.C. Chen <yacheng600...@gmail.com> wrote: > > > > From: "Y.C. Chen" <yc_c...@aspeedtech.com> > > > > v1: over-sample data to increase the stability with some specific > monitors > > v2: refine to avoid infinite loop > > This contains at least 4 whitespace breakages (val =) in a few > places, also why the volatiles, I don't think they make much sense > here, have you verified they are required? > > Dave. > > > > > Signed-off-by: Y.C. Chen <yc_c...@aspeedtech.com> > > --- > > drivers/gpu/drm/ast/ast_mode.c | 34 ++++++++++++++++++++++++++++------ > > 1 file changed, 28 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/ast/ast_mode.c > b/drivers/gpu/drm/ast/ast_mode.c > > index 5e77d45..092e9a7 100644 > > --- a/drivers/gpu/drm/ast/ast_mode.c > > +++ b/drivers/gpu/drm/ast/ast_mode.c > > @@ -972,9 +972,20 @@ static int get_clock(void *i2c_priv) > > { > > struct ast_i2c_chan *i2c = i2c_priv; > > struct ast_private *ast = i2c->dev->dev_private; > > - uint32_t val; > > + volatile uint32_t val, val2, count, pass; > > + > > + count = 0; > > + pass = 0; > > + val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, > 0x10) >> 4) & 0x01; > > + do { > > + val2 = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, > 0xb7, 0x10) >> 4) & 0x01; > > + if (val == val2) pass++; > > + else { > > + pass = 0; > > + val = (ast_get_index_reg_mask(ast, > AST_IO_CRTC_PORT, 0xb7, 0x10) >> 4) & 0x01; > > > > > > + } > > + } while ((pass < 5) && (count++ < 0x10000)); > > > > - val = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x10) > >> 4; > > return val & 1 ? 1 : 0; > > } > > > > @@ -982,9 +993,20 @@ static int get_data(void *i2c_priv) > > { > > struct ast_i2c_chan *i2c = i2c_priv; > > struct ast_private *ast = i2c->dev->dev_private; > > - uint32_t val; > > + volatile uint32_t val, val2, count, pass; > > + > > + count = 0; > > + pass = 0; > > + val = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, > 0x20) >> 5) & 0x01; > > + do { > > + val2 = (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, > 0xb7, 0x20) >> 5) & 0x01; > > + if (val == val2) pass++; > > + else { > > + pass = 0; > > + val = (ast_get_index_reg_mask(ast, > AST_IO_CRTC_PORT, 0xb7, 0x20) >> 5) & 0x01; > > + } > > + } while ((pass < 5) && (count++ < 0x10000)); > > > > - val = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, 0x20) > >> 5; > > return val & 1 ? 1 : 0; > > } > > > > @@ -997,7 +1019,7 @@ static void set_clock(void *i2c_priv, int clock) > > > > for (i = 0; i < 0x10000; i++) { > > ujcrb7 = ((clock & 0x01) ? 0 : 1); > > - ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, > 0xfe, ujcrb7); > > + ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, > 0xf4, ujcrb7); > > jtemp = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, > 0xb7, 0x01); > > if (ujcrb7 == jtemp) > > break; > > @@ -1013,7 +1035,7 @@ static void set_data(void *i2c_priv, int data) > > > > for (i = 0; i < 0x10000; i++) { > > ujcrb7 = ((data & 0x01) ? 0 : 1) << 2; > > - ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, > 0xfb, ujcrb7); > > + ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb7, > 0xf1, ujcrb7); > > jtemp = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, > 0xb7, 0x04); > > if (ujcrb7 == jtemp) > > break; > > -- > > 1.8.3.1 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel